[PATCH] D10839: code refactoring on ARMTargetInfo class
Alexandros Lamprineas
alexandros.lamprineas at arm.com
Mon Jul 13 07:12:37 PDT 2015
labrinea added inline comments.
================
Comment at: lib/Basic/Targets.cpp:4222
@@ +4221,3 @@
+ setArchInfo(DefaultCPU);
+ ShouldUseInlineAtomic = (ArchISA == llvm::ARM::IK_ARM &&
+ ArchVersion >= 6) ||
----------------
rengolin wrote:
> ShouldUseInlineAtomic must be set up inside setAtomic(), too.
Moving it there triggers regression (tools/clang/test/CodeGen/atomics-inlining.c). When triple does not define a subArch, (and therefore llvm::ARMTargetParser::getDefaultCPU(ArchName) returns null), the value of ShouldUseInlineAtomic should be **false** according to the logic before my patch. That's because ShouldUseInlineAtomic was parsing the arch name and returned true only if archName started with "arm**v**" or "thumb**v**".
================
Comment at: lib/Basic/Targets.cpp:4228
@@ +4227,3 @@
+ else {
+ setArchInfo(CPU);
+ ShouldUseInlineAtomic = false;
----------------
If we are finally keeping the hard-coded string for CPU with a FIXME label, there is no point in having setArchInfo(ArchName). It is redundant. We still have to call setArchInfo(CPU) to avoid the regressions.
http://reviews.llvm.org/D10839
More information about the cfe-commits
mailing list