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 _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
