=?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]>
spall wrote: > I have done that, but the fallout is 66 tests. 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. > > 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. > > 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? Hi from the HLSL team. I worked on adding support for HLSLs unpacked bool vectors and have been eagerly waiting for this PR to merge. We want to use a comparison to zero instead of a truncation to convert from an i32 bool to an i1 bool. The test changes in the diff looked okay to me. And for unpacked bool vectors we would always want to do a comparison to zero instead of a truncation. https://github.com/llvm/llvm-project/pull/160790 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
