[PATCH] D124368: [LoopInstSimplify] Ignore users in unreachable blocks. PR55072

[Florian Hahn via Phabricator via llvm-commits]

fhahn accepted this revision.
fhahn added a comment.
This revision is now accepted and ready to land.

LGTM, thanks! I think it would be good to remove the branch on undef, as per inline comment and possible remove some of blocks that seem unneeded.



================
Comment at: llvm/test/Transforms/LoopInstSimplify/pr55072.ll:6
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128-ni:1-p2:32:8:8:32-ni:2"
 target triple = "x86_64-unknown-linux-gnu"
 
----------------
nit: the test shouldn't be X86 specific, can this be removed?


================
Comment at: llvm/test/Transforms/LoopInstSimplify/pr55072.ll:39
 
 bb2:                                              ; preds = %bb2
   br label %bb2
----------------
Those branches look like an artifact of `llvm-reduce`, where it fails to remove them. Are they needed?


================
Comment at: llvm/test/Transforms/LoopInstSimplify/pr55072.ll:43
 bb3:                                              ; preds = %bb8, %bb1
   switch i32 undef, label %bb8 [
+  i32 0, label %bb4
----------------
Avoiding switching on undef might make the test less prone to unnecessary changes in case  loop-instsimplify becomes more powerful in the future. 

Branch/switch on undef is UB and if the source has UB this will mean verification with tools like Alive2 will always succeed, regardless of what the result of the transform is.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124368/new/

https://reviews.llvm.org/D124368