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




_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to