[llvm] [DAG] Add freeze(assertext(x)) -> assertext(x) folds (PR #94491)

Björn Pettersson via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 6 01:21:48 PDT 2024


================
@@ -57,7 +57,10 @@ enum NodeType {
   /// been extended, and the second is a value type node indicating the width
   /// of the extension.
   /// NOTE: In case of the source value (or any vector element value) is
-  /// poisoned the assertion will not be true for that value.
+  /// poisoned the assertion will not be true for that value and the
+  /// corresponding result value will be poison. If a source value isn't
+  /// satisfying the condition being asserted (while not being poison), then
+  /// this is considered as immediate undefined behavior.
----------------
bjope wrote:

I thought that the fptoi legalization fulfilled this patch. That either the fptoi result is poison (and then it is still poison after the assert), or the result of fptoi would satisfy the assert. This patch is at least not breaking the regression tests related to https://github.com/llvm/llvm-project/issues/66603 .

But maybe this whole idea isn't working anyway. Not sure how to protect from further simplifications of the code leading up to the assert, that could make it less poisonous. It would for example not be legal to just freeze the FP_TO_SINT introduced by legalization, without making sure that the frozen value fulfil the later assert.
So just defining it as immediate UB is probably a lot more complicated than this.

Now I'm just back into the mega-confused-state. And I can't really see how we possibly can handle the (freeze(assertext)) kind of regressions seen in https://github.com/llvm/llvm-project/pull/84924 . Maybe if we try to move the freeze in the other direction when there is an assert (or at least avoid moving it as aggressively in that direction when it might block optimizations.

https://github.com/llvm/llvm-project/pull/94491


More information about the llvm-commits mailing list