[PATCH] D96083: [LTT] Don't attempt to lower type tests used only by assumes

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

Fixed most of the suggestions at a80232bd5f125ab81554882825a564bdc7ab8e0c <https://reviews.llvm.org/rGa80232bd5f125ab81554882825a564bdc7ab8e0c>. Question about the other one below.



================
Comment at: llvm/lib/Transforms/IPO/LowerTypeTests.cpp:2067
+      // with the DropTypeTests flag set.
+      bool OnlyAssumeUses = !CI->use_empty();
+      for (auto CIU = CI->use_begin(), CIUE = CI->use_end(); CIU != CIUE;) {
----------------
tejohnson wrote:
> pcc wrote:
> > If the intrinsic is unused then we don't want to rewrite it either so you can initialize this to true.
> I tried that but it broke a half dozen or so LTT tests (e.g. llvm/test/Transforms/LowerTypeTests/layout.ll) where there were type tests and no uses of the results. The other possibility was to change those tests, but I thought with this initial value it would simplify creating tests. WDYT?
I could change this if I made fixes like this one to ~6 of the LTT tests to make sure the type test results are used before invoking the LTT pass on its own via opt. Let me know what you prefer to do here. Not sure if this makes it more of a pain to test?


```
diff --git a/llvm/test/Transforms/LowerTypeTests/layout.ll b/llvm/test/Transforms/LowerTypeTests/layout.ll
index 7075955790d9..38fe5be10aad 100644
--- a/llvm/test/Transforms/LowerTypeTests/layout.ll
+++ b/llvm/test/Transforms/LowerTypeTests/layout.ll
@@ -18,10 +18,12 @@ target datalayout = "e-p:32:32"
 !2 = !{i32 0, !"typeid3"}
 
 declare i1 @llvm.type.test(i8* %ptr, metadata %bitset) nounwind readnone
+declare void @bar(i1 %x, i1 %y, i1 %z)
 
 define void @foo() {
   %x = call i1 @llvm.type.test(i8* undef, metadata !"typeid1")
   %y = call i1 @llvm.type.test(i8* undef, metadata !"typeid2")
   %z = call i1 @llvm.type.test(i8* undef, metadata !"typeid3")
+  call void @bar(i1 %x, i1 %y, i1 %z)
   ret void
 }
```




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96083