=?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]>


apple-fcloutier 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?

https://github.com/llvm/llvm-project/pull/160790
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to