probinson added inline comments.
================ Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5596 + DefaultedSMFArePOD = false; + } + ---------------- 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! 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
