llvm.org GIT mirror llvm / 62616fc
[ARM] Fix bugs introduced by the fp64/d32 rework. Change D60691 caused some knock-on failures that weren't caught by the existing tests. Firstly, selecting a CPU that should have had a restricted FPU (e.g. `-mcpu=cortex-m4`, which should have 16 d-regs and no double precision) could give the unrestricted version, because `ARM::getFPUFeatures` returned a list of features including subtracted ones (here `-fp64`,`-d32`), but `ARMTargetInfo::initFeatureMap` threw away all the ones that didn't start with `+`. Secondly, the preprocessor macros didn't reliably match the actual compilation settings: for example, `-mfpu=softvfp` could still set `__ARM_FP` as if hardware FP was available, because the list of features on the cc1 command line would include things like `+vfp4`,`-vfp4d16` and clang didn't realise that one of those cancelled out the other. I've fixed both of these issues by rewriting `ARM::getFPUFeatures` so that it returns a list that enables every FP-related feature compatible with the selected FPU and disables every feature not compatible, which is more verbose but means clang doesn't have to understand the dependency relationships between the backend features. Meanwhile, `ARMTargetInfo::handleTargetFeatures` is testing for all the various forms of the FP feature names, so that it won't miss cases where it should have set `HW_FP` to feed into feature test macros. That in turn caused an ordering problem when handling `-mcpu=foo+bar` together with `-mfpu=something_that_turns_off_bar`. To fix that, I've arranged that the `+bar` suffixes on the end of `-mcpu` and `-march` cause feature names to be put into a separate vector which is concatenated after the output of `getFPUFeatures`. Another side effect of all this is to fix a bug where `clang -target armv8-eabi` by itself would fail to set `__ARM_FEATURE_FMA`, even though `armv8` (aka Arm v8-A) implies FP-Armv8 which has FMA. That was because `HW_FP` was being set to a value including only the `FPARMV8` bit, but that feature test macro was testing only the `VFP4FPU` bit. Now `HW_FP` ends up with all the bits set, so it gives the right answer. Changes to tests included in this patch: * `arm-target-features.c`: I had to change basically all the expected results. (The Cortex-M4 test in there should function as a regression test for the accidental double-precision bug.) * `arm-mfpu.c`, `armv8.1m.main.c`: switched to using `CHECK-DAG` everywhere so that those tests are no longer sensitive to the order of cc1 feature options on the command line. * `arm-acle-6.5.c`: been updated to expect the right answer to that FMA test. * `Preprocessor/arm-target-features.c`: added a regression test for the `mfpu=softvfp` issue. Reviewers: SjoerdMeijer, dmgreen, ostannard, samparker, JamesNagurne Reviewed By: ostannard Subscribers: srhines, javed.absar, kristof.beyls, hiraditya, cfe-commits, llvm-commits Tags: #clang, #llvm Differential Revision: https://reviews.llvm.org/D62998 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@362791 91177308-0d34-0410-b5e6-96231b3b80d8 Simon Tatham 5 months ago
1 changed file(s) with 57 addition(s) and 81 deletion(s). Raw diff Collapse all Expand all
161161 if (FPUKind >= FK_LAST || FPUKind == FK_INVALID)
162162 return false;
163163
164 // FPU version subtarget features are inclusive of lower-numbered ones, so
165 // enable the one corresponding to this version and disable all that are
166 // higher. We also have to make sure to disable fp16 when vfp4 is disabled,
167 // as +vfp4 implies +fp16 but -vfp4 does not imply -fp16.
168 switch (FPUNames[FPUKind].FPUVer) {
169 case FPUVersion::VFPV5_FULLFP16:
170 Features.push_back("+fp-armv8");
171 Features.push_back("+fullfp16");
172 break;
173 case FPUVersion::VFPV5:
174 Features.push_back("+fp-armv8");
175 break;
176 case FPUVersion::VFPV4:
177 Features.push_back("+vfp4");
178 Features.push_back("-fp-armv8");
179 break;
180 case FPUVersion::VFPV3_FP16:
181 Features.push_back("+vfp3");
182 Features.push_back("+fp16");
183 Features.push_back("-vfp4");
184 Features.push_back("-fp-armv8");
185 break;
186 case FPUVersion::VFPV3:
187 Features.push_back("+vfp3");
188 Features.push_back("-fp16");
189 Features.push_back("-vfp4");
190 Features.push_back("-fp-armv8");
191 break;
192 case FPUVersion::VFPV2:
193 Features.push_back("+vfp2");
194 Features.push_back("-vfp3");
195 Features.push_back("-fp16");
196 Features.push_back("-vfp4");
197 Features.push_back("-fp-armv8");
198 break;
199 case FPUVersion::NONE:
200 Features.push_back("-fpregs");
201 Features.push_back("-vfp2");
202 Features.push_back("-vfp3");
203 Features.push_back("-fp16");
204 Features.push_back("-vfp4");
205 Features.push_back("-fp-armv8");
206 break;
207 }
208
209 // fp64 and d32 subtarget features are independent of each other, so we
210 // must disable/enable both.
211 if (FPUKind == FK_NONE) {
212 Features.push_back("-fp64");
213 Features.push_back("-d32");
214 } else {
215 switch (FPUNames[FPUKind].Restriction) {
216 case FPURestriction::SP_D16:
217 Features.push_back("-fp64");
218 Features.push_back("-d32");
219 break;
220 case FPURestriction::D16:
221 Features.push_back("+fp64");
222 Features.push_back("-d32");
223 break;
224 case FPURestriction::None:
225 Features.push_back("+fp64");
226 Features.push_back("+d32");
227 break;
228 }
229 }
230
231 // crypto includes neon, so we handle this similarly to FPU version.
232 switch (FPUNames[FPUKind].NeonSupport) {
233 case NeonSupportLevel::Crypto:
234 Features.push_back("+neon");
235 Features.push_back("+crypto");
236 break;
237 case NeonSupportLevel::Neon:
238 Features.push_back("+neon");
239 Features.push_back("-crypto");
240 break;
241 case NeonSupportLevel::None:
242 Features.push_back("-neon");
243 Features.push_back("-crypto");
244 break;
164 static const struct FPUFeatureNameInfo {
165 const char *PlusName, *MinusName;
166 FPUVersion MinVersion;
167 FPURestriction MaxRestriction;
168 } FPUFeatureInfoList[] = {
169 // We have to specify the + and - versions of the name in full so
170 // that we can return them as static StringRefs.
171 //
172 // Also, the SubtargetFeatures ending in just "sp" are listed here
173 // under FPURestriction::None, which is the only FPURestriction in
174 // which they would be valid (since FPURestriction::SP doesn't
175 // exist).
176
177 {"+fpregs", "-fpregs", FPUVersion::VFPV2, FPURestriction::SP_D16},
178 {"+vfp2", "-vfp2", FPUVersion::VFPV2, FPURestriction::None},
179 {"+vfp2d16", "-vfp2d16", FPUVersion::VFPV2, FPURestriction::D16},
180 {"+vfp2d16sp", "-vfp2d16sp", FPUVersion::VFPV2, FPURestriction::SP_D16},
181 {"+vfp2sp", "-vfp2sp", FPUVersion::VFPV2, FPURestriction::None},
182 {"+vfp3", "-vfp3", FPUVersion::VFPV3, FPURestriction::None},
183 {"+vfp3d16", "-vfp3d16", FPUVersion::VFPV3, FPURestriction::D16},
184 {"+vfp3d16sp", "-vfp3d16sp", FPUVersion::VFPV3, FPURestriction::SP_D16},
185 {"+vfp3sp", "-vfp3sp", FPUVersion::VFPV3, FPURestriction::None},
186 {"+fp16", "-fp16", FPUVersion::VFPV3_FP16, FPURestriction::SP_D16},
187 {"+vfp4", "-vfp4", FPUVersion::VFPV4, FPURestriction::None},
188 {"+vfp4d16", "-vfp4d16", FPUVersion::VFPV4, FPURestriction::D16},
189 {"+vfp4d16sp", "-vfp4d16sp", FPUVersion::VFPV4, FPURestriction::SP_D16},
190 {"+vfp4sp", "-vfp4sp", FPUVersion::VFPV4, FPURestriction::None},
191 {"+fp-armv8", "-fp-armv8", FPUVersion::VFPV5, FPURestriction::None},
192 {"+fp-armv8d16", "-fp-armv8d16", FPUVersion::VFPV5, FPURestriction::D16},
193 {"+fp-armv8d16sp", "-fp-armv8d16sp", FPUVersion::VFPV5, FPURestriction::SP_D16},
194 {"+fp-armv8sp", "-fp-armv8sp", FPUVersion::VFPV5, FPURestriction::None},
195 {"+fullfp16", "-fullfp16", FPUVersion::VFPV5_FULLFP16, FPURestriction::SP_D16},
196 {"+fp64", "-fp64", FPUVersion::VFPV2, FPURestriction::D16},
197 {"+d32", "-d32", FPUVersion::VFPV2, FPURestriction::None},
198 };
199
200 for (const auto &Info: FPUFeatureInfoList) {
201 if (FPUNames[FPUKind].FPUVer >= Info.MinVersion &&
202 FPUNames[FPUKind].Restriction <= Info.MaxRestriction)
203 Features.push_back(Info.PlusName);
204 else
205 Features.push_back(Info.MinusName);
206 }
207
208 static const struct NeonFeatureNameInfo {
209 const char *PlusName, *MinusName;
210 NeonSupportLevel MinSupportLevel;
211 } NeonFeatureInfoList[] = {
212 {"+neon", "-neon", NeonSupportLevel::Neon},
213 {"+crypto", "-crypto", NeonSupportLevel::Crypto},
214 };
215
216 for (const auto &Info: NeonFeatureInfoList) {
217 if (FPUNames[FPUKind].NeonSupport >= Info.MinSupportLevel)
218 Features.push_back(Info.PlusName);
219 else
220 Features.push_back(Info.MinusName);
245221 }
246222
247223 return true;