[InstCombine] Don't let an alignment assume prevent new/delete removals.

[hjyamauchi]

Summary

Remove allocations with alignment assume.

[davidxl]

Can this be generalized to all assumes? nor just the code pattern that checks alignment?

[lebedev.ri]

This is really a temporary workaround until bundle representation of assumptions is used.
It might be better to work on that instead
See also D75462.

[jdoerfert]

This is the right fix for alignment: D71739

For all assumes (and later other stuff, think lifetime markers or annotations) you can check if the User::isDroppable, and eventually drop via Value::dropDroppableUses if you cannot update them properly.

[hjyamauchi]

In D81854#2096685, @jdoerfert wrote:

This is the right fix for alignment: D71739

For all assumes (and later other stuff, think lifetime markers or annotations) you can check if the User::isDroppable, and eventually drop via Value::dropDroppableUses if you cannot update them properly.

@lebedev.ri, @jdoerfert: I acknowledge D71739. What's the status of it? Are you open to having this patch as a temporary measure until that's checked in?

[davidxl]

perhaps add a FIXME comment to point to the related patch and final state.

[jdoerfert]

In D81854#2098885, @yamauchi wrote:

In D81854#2096685, @jdoerfert wrote:

This is the right fix for alignment: D71739

For all assumes (and later other stuff, think lifetime markers or annotations) you can check if the User::isDroppable, and eventually drop via Value::dropDroppableUses if you cannot update them properly.

@lebedev.ri, @jdoerfert: I acknowledge D71739. What's the status of it? Are you open to having this patch as a temporary measure until that's checked in?

I pinged the other patch. It is basically ready, minor changes only. Let's wait a day or so and see if @Tyker responds.

[jdoerfert]

@Tyker merged D71739, if it sticks, can you check if this is still necessary?

[hjyamauchi]

Update past D71739.

[hjyamauchi]

In D81854#2114240, @jdoerfert wrote:

@Tyker merged D71739, if it sticks, can you check if this is still necessary?

Thanks. This is still necessary for this particular alloc removal case, but D71739 made it much simpler.

[davidxl]

lgtm

[jdoerfert]

Looks much better indeed ;)

Please add a commit message.

[hjyamauchi]

Update a test which was somehow missed.

[hjyamauchi]

PTAL.

[davidxl]

llvm/test/Transforms/PhaseOrdering/inlining-alignment-assumptions.ll

101 ↗

(On Diff #274180)

what is this change about?

[hjyamauchi]

llvm/test/Transforms/PhaseOrdering/inlining-alignment-assumptions.ll

101 ↗

(On Diff #274180)

This is another example that this patch improves (fixing a problem that an assume was preventing the removal of an alloca).

[davidxl]

llvm/test/Transforms/PhaseOrdering/inlining-alignment-assumptions.ll

88 ↗

(On Diff #274180)

This comment is out of date then

[hjyamauchi]

Address comment.

llvm/test/Transforms/PhaseOrdering/inlining-alignment-assumptions.ll

88 ↗

(On Diff #274180)

Updated.

[davidxl]

lgtm

llvm/lib/Transforms/InstCombine/InstructionCombining.cpp

for (User *U : PI->users()) {
           case Intrinsic::memmove:
           case Intrinsic::memcpy:
           case Intrinsic::memset: {
             MemIntrinsic *MI = cast<MemIntrinsic>(II);
             if (MI->isVolatile() || MI->getRawDest() != PI)
               return false;
             LLVM_FALLTHROUGH;
           }
+          case Intrinsic::assume:
           case Intrinsic::invariant_start:
           case Intrinsic::invariant_end:
           case Intrinsic::lifetime_start:
           case Intrinsic::lifetime_end:
           case Intrinsic::objectsize:
             Users.emplace_back(I);
             continue;
           }

llvm/test/Transforms/InstCombine/malloc-free-delete.ll

 declare void @_ZdlPvjSt11align_val_t(i8*, i32, i32) nobuiltin
 ; delete(void*, unsigned long, align_val_t)
 declare void @_ZdlPvmSt11align_val_t(i8*, i64, i64) nobuiltin
 ; delete[](void*, unsigned int, align_val_t)
 declare void @_ZdaPvjSt11align_val_t(i8*, i32, i32) nobuiltin
 ; delete[](void*, unsigned long, align_val_t)
 declare void @_ZdaPvmSt11align_val_t(i8*, i64, i64) nobuiltin
 
+declare void @llvm.assume(i1)
+
 define void @test8() {
 ; CHECK-LABEL: @test8(
 ; CHECK-NEXT:    ret void
 ;
   %nt = alloca i8
   %nw = call i8* @_Znwm(i64 32) builtin
   call void @_ZdlPv(i8* %nw) builtin
   %na = call i8* @_Znam(i64 32) builtin
;
   %nwa2 = call i8* @_ZnwmSt11align_val_t(i64 32, i64 8) builtin
   call void @_ZdlPvmSt11align_val_t(i8* %nwa2, i64 32, i64 8) builtin
   %nwja2 = call i8* @_ZnwjSt11align_val_t(i32 32, i32 8) builtin
   call void @_ZdlPvjSt11align_val_t(i8* %nwa2, i32 32, i32 8) builtin
   %naa2 = call i8* @_ZnamSt11align_val_t(i64 32, i64 8) builtin
   call void @_ZdaPvmSt11align_val_t(i8* %naa2, i64 32, i64 8) builtin
   %naja2 = call i8* @_ZnajSt11align_val_t(i32 32, i32 8) builtin
   call void @_ZdaPvjSt11align_val_t(i8* %naja2, i32 32, i32 8) builtin
+
+  ; Check that the alignment assume does not prevent the removal.
+  %nwa3 = call i8* @_ZnwmSt11align_val_t(i64 32, i64 16) builtin
+  call void @llvm.assume(i1 true) [ "align"(i8* %nwa3, i64 16) ]
+  call void @_ZdlPvmSt11align_val_t(i8* %nwa3, i64 32, i64 16) builtin
+
   ret void
 }
 
 declare noalias i8* @"\01??2@YAPEAX_K@Z"(i64) nobuiltin
 declare void @"\01??3@YAXPEAX@Z"(i8*) nobuiltin
 
 define void @test9() {
 ; CHECK-LABEL: @test9(

 ; CHECK-NEXT:    ret void
 ;
   call void @_ZdlPv(i8* null)
   ret void
 }
 
 define void @test11() {
 ; CHECK-LABEL: @test11(
-; CHECK-NEXT:    [[CALL:%.*]] = call dereferenceable(8) i8* @_Znwm(i64 8) #6
+; CHECK-NEXT:    [[CALL:%.*]] = call dereferenceable(8) i8* @_Znwm(i64 8) #7
 ; CHECK-NEXT:    call void @_ZdlPv(i8* nonnull [[CALL]])
 ; CHECK-NEXT:    ret void
 ;
   %call = call i8* @_Znwm(i64 8) builtin
   call void @_ZdlPv(i8* %call)
   ret void
 }