dblaikie added inline comments.
================ Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5596 + DefaultedSMFArePOD = false; + } + ---------------- probinson wrote: > dblaikie wrote: > > probinson wrote: > > > dblaikie wrote: > > > > probinson wrote: > > > > > Does this mean `-fclang-abi-compat` will override the PS4/Darwin > > > > > special case? I think we don't want to do that. > > > > I don't think so - this expression will make `DefaultedSMFArePOD` false > > > > when it's already false due to the target not being PS4 or Darwin, > > > > yeah? So it'll redundantly/harmlessly set it to false. > > > No, I mean if it *is* PS4, it will turn true into false, and that's bad. > > > If I'm reading this correctly. > > Right - let's see if I can explain how I'm understanding it at least - > > maybe I've got some code blindness from staring at it too long. > > > > If it /is/ PS4, then this code: > > ``` > > bool DefaultedSMFArePOD = !RawTriple.isPS4() && !RawTriple.isOSDarwin(); > > ``` > > Will amount to this: > > ``` > > bool DefaultedSMFArePOD = !true && !RawTriple.isOSDarwin(); > > ``` > > ``` > > bool DefaultedSMFArePOD = false && !RawTriple.isOSDarwin(); > > ``` > > ``` > > bool DefaultedSMFArePOD = false; > > ``` > > So then then the code at 5595 can only reinforce that, not change it - > > since it only sets the value to false. So my understanding is that the > > `-fclang-abi-compat` flag can not have any effect on the > > `DefaultedSMFArePOD` behavior of PS4. And it sounds like that's what you > > want? So I think we're good here? > Those little bangs can be hard to see sometimes... Also, I think this bit > > already false due to the target not being PS4 or Darwin, > confused me, should have read > > already false due to the target being PS4 or Darwin, > and so what the code actually does is in fact what we want. > > Thanks for your patience in explaining it, and I will go make an eye doctor > appointment! > I think this bit > > already false due to the target not being PS4 or Darwin, > confused me, should have read > > already false due to the target being PS4 or Darwin, Ah, right you are, sorry about that! > Thanks for your patience in explaining it, and I will go make an eye doctor > appointment! Sure thing - certainly sometimes consider whether or not switching to the C++ alternative tokens would be an improvement... ``` DefaultedSMFArePOD = not RawTriple.isPS4() and not RawTriple.isOSDarwin() ``` ;) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119051/new/ https://reviews.llvm.org/D119051 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
