[PATCH] D10839: code refactoring on ARMTargetInfo class

Alexandros Lamprineas alexandros.lamprineas at arm.com
Mon Jul 13 07:38:20 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:
> labrinea wrote:
> > 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**".
> But by the time you run setAtomic() you already have Arch, CPU and everything. And if you don't, then check for nullptr and set atomic to false.
The tricky part is that these variables (Arch, CPU, etc..) are set both in the **if** and the **else** path, regardless if the triple specifies a sub arch or not. What I could do is to cache DefaultCPU, which is set by llvm::ARMTargetParser::getDefaultCPU(ArchName), and test that for nullptr value in setAtomic().


http://reviews.llvm.org/D10839







More information about the cfe-commits mailing list