[PATCH] D10839: code refactoring on ARMTargetInfo class

[Renato Golin]

rengolin added inline comments.

================
Comment at: lib/Basic/Targets.cpp:4222
@@ +4221,3 @@
+      setArchInfo(DefaultCPU);
+      ShouldUseInlineAtomic = (ArchISA == llvm::ARM::IK_ARM &&
+                                          ArchVersion >= 6) ||
----------------
labrinea wrote:
> 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().
Yes, that could work.


http://reviews.llvm.org/D10839