=?utf-8?q?Félix?= Cloutier <[email protected]>, =?utf-8?q?Félix?= Cloutier <[email protected]>, =?utf-8?q?Félix?= Cloutier <[email protected]>, =?utf-8?q?Félix?= Cloutier <[email protected]> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/[email protected]>
AaronBallman wrote: > I have done that, but the fallout is 66 tests. Oof. > I am more concerned that something will go wrong and the change will need to > be reverted. In order to reduce the amount of work I'm exposed to if things > do go wrong, I am pushing the change to a different branch and I would like > us to merge it separately from this one. That way, if there's a problem, we > can revert the change to -O0 codegen without pulling out -fstrict-bool. The > diff is > [here](https://github.com/swiftlang/llvm-project/compare/apple-fcloutier/139397212...swiftlang:llvm-project:apple-fcloutier/139397212-O0?expand=1). > I am especially concerned about changes in HLSL tests because I don't know > if it's disturbing a load-bearing bit of codegen for them. I think that approach makes a lot of sense. CC @llvm-beanz for some eyeballs on the HLSL tests in that branch, because I also don't know HLSL well enough to answer that. CC @efriedma-quic @rjmccall for CodeGen in general, but my looking over the tests in the diff didn't spot anything that looked suspicious or wrong. > Changing the tests also made me realize that I didn't change the `bool` > vector case from `trunc <N x i8> to <N x i1>` to `icmp ne` because Clang does > not attach range metadata to bool vector values. However, if O0 codegen is > changing to match, it might mean we should also change the vector codegen. > This is outside of what I'm familiar with, so I'd like to see if you think > this is a good idea too. Good catch; my intuition is that vector bool operations should have the same semantics as scalar bool operations, just...vectorized. But vector operations may have different costs associated with them and so maybe deviation is warranted? I'm not certain. > Here's what I think comes next: > > * If we think bool vector codegen needs to follow > -fno-strict-bool=nonzero rules (currently it _always_ truncates), then I will > make the additional change before we merge. > > * If we want to change our mind about nonzero because of -O0, then I will > make that change before we merge too. > > * If we want to try the -O0 change, we should merge this change the way > it is and follow up on the other change (for which I already have the branch > out, so I'm not doing this because I'm trying to run away 🙂) so that we can > limit the revert damage if something breaks. > > > How does that sound to you? I think this sounds like a reasonable path forward. As for the vector changes, I think they should honor `-fno-strict-bool=nonzero` as part of this PR, but we don't need to change the default behavior for them (we'll figure that out in the `-O0` discussion). CC @fhahn for some opinions on the vector types (maybe he'll know a better person to rope in for that) because they're outside of my wheelhouse. https://github.com/llvm/llvm-project/pull/160790 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
