[clang] [clang] Implement -fstrict-bool (PR #160790)

[Sarah Spall via cfe-commits]

=?utf-8?q?Félix?= Cloutier <fcloutier at apple.com>,
=?utf-8?q?Félix?= Cloutier <fcloutier at apple.com>,
=?utf-8?q?Félix?= Cloutier <fcloutier at apple.com>,
=?utf-8?q?Félix?= Cloutier <fcloutier at apple.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/160790 at github.com>


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