[InstCombine] inttoptr(load) -> load

[lebedev.ri]

Summary

See D88789 / D88788 / D88806 / D75505.

[lebedev.ri]

llvm/test/Transforms/InstCombine/select.ll

1145–1146

Missing fold

[efriedma]

I guess the idea makes sense.

Not sure what the isUnordered() check is doing. And this needs to be address-space aware.

llvm/utils/compare-stats.py

2

Did you mean to include this?

[lebedev.ri]

In D88842#2312448, @efriedma wrote:

I guess the idea makes sense.

Aha, finally we're getting somewhere!

Not sure what the isUnordered() check is doing.

This is the restriction on combineLoadToOperationType().
Can we do this unconditionally?

And this needs to be address-space aware.

combineLoadToNewType() already takes care of annotating the new load type with proper original address space, so we should be good.

llvm/utils/compare-stats.py

2

Err, no

[jdoerfert]

In D88842#2312459, @lebedev.ri wrote:

In D88842#2312448, @efriedma wrote:

Not sure what the isUnordered() check is doing.

This is the restriction on combineLoadToOperationType().
Can we do this unconditionally?

I think we should not do this for atomic operations if the bitwidth is potentially different. That said, I don't see an immediate reason for the type to make an impact otherwise.

[lebedev.ri]

In D88842#2312467, @jdoerfert wrote:

In D88842#2312459, @lebedev.ri wrote:

In D88842#2312448, @efriedma wrote:

Not sure what the isUnordered() check is doing.

This is the restriction on combineLoadToOperationType().
Can we do this unconditionally?

I think we should not do this for atomic operations if the bitwidth is potentially different. That said, I don't see an immediate reason for the type to make an impact otherwise.

The bitwidth is not different, because we've just fixed bitwidth mismatch in the fold above.
Therefore i'll make this unconditional.

[lebedev.ri]

Don't restrict to simple/unordered loads.

[nlopes]

I don't like the direction of this patch because it will remove inttoptr instructions that were present in the original program. In the same way that optimizations shouldn't introduce inttoptr, they shouldn't fuse them with loads either.
If the original program had the inttoptr cast and you fold it with a load instruction, then you are asking the load to do the cast. This sort of type punning is not ok. I strongly disagree with Chandler that LLVM's memory is not typed. It needs to distinguish between integers and pointers, otherwise I don't know how to make LLVM correct without a significant perf penalty.
If LLVM's memory was untyped, we would have to assume that every pointer load could be doing an implicit inttoptr cast, which would havoc most optimizations. The fact that some optimizations, like sroa, assume that the memory is untyped and most of the others don't is a source of miscompilations.

[lebedev.ri]

Thanks @nlopes!
This really should not be a comment, but in some actual docs, like langref.
While there, could you please also comment on D88788 and D88806?

llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp

if (CI.getOperand(0)->getType()->getScalarSizeInBits() !=
 
     Value *P = Builder.CreateZExtOrTrunc(CI.getOperand(0), Ty);
     return new IntToPtrInst(P, CI.getType());
   }
 
   if (Instruction *I = commonCastTransforms(CI))
     return I;
 
+  if (auto *LI = dyn_cast<LoadInst>(CI.getOperand(0))) {
+    if (LI->isUnordered()) {
+      // inttoptr/ptrtoint casts are Bad News, and in general, there is little
+      // we can do about them. However, casting pointer to an integer is,
+      // arguably, less troubling than casting integer to a pointer.
+      // So if we have a inttoptr(load), let's aggressively fold that to load
+      // of a pointer, and adjust *all* users ourselves.
+      Builder.SetInsertPoint(LI);
+      LoadInst *NewLoad =
+          combineLoadToNewType(*LI, PointerType::getInt8PtrTy(LI->getContext()),
+                               LI->getName() + ".ptr");
+      replaceInstUsesWith(
+          CI, Builder.CreatePointerCast(NewLoad, CI.getType(), CI.getName()));
+      if (!LI->use_empty()) {
+        // Original load had other uses besides our original inttoptr cast,
+        // so we have to update them as well.
+        auto *P2I = Builder.CreatePointerCast(NewLoad, LI->getType());
+        replaceInstUsesWith(*LI, P2I);
+      }
+      eraseInstFromFunction(*LI);
+      return nullptr;
+    }
+  }
+
   return nullptr;
 }
 
 /// Implement the transforms for cast of pointer (bitcast/ptrtoint)
 Instruction *InstCombinerImpl::commonPointerCastTransforms(CastInst &CI) {
   Value *Src = CI.getOperand(0);
 
   if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(Src)) {

llvm/test/Transforms/InstCombine/multi-use-load-casts.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -S -instcombine < %s | FileCheck %s
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 
 ; Positive test - all uses are identical casts.
 define void @t0(i1 zeroext %c0, i1 zeroext %c1, i64* nocapture readonly %src) {
 ; CHECK-LABEL: @t0(
 ; CHECK-NEXT:  bb:
-; CHECK-NEXT:    [[DATA:%.*]] = load i64, i64* [[SRC:%.*]], align 8
+; CHECK-NEXT:    [[TMP0:%.*]] = bitcast i64* [[SRC:%.*]] to i8**
+; CHECK-NEXT:    [[DATADATA_PTR:%.*]] = load i8*, i8** [[TMP0]], align 8
 ; CHECK-NEXT:    br i1 [[C0:%.*]], label [[BB3:%.*]], label [[BB7:%.*]]
 ; CHECK:       bb3:
 ; CHECK-NEXT:    br i1 [[C1:%.*]], label [[BB4:%.*]], label [[BB5:%.*]]
 ; CHECK:       bb4:
 ; CHECK-NEXT:    tail call void @abort()
 ; CHECK-NEXT:    unreachable
 ; CHECK:       bb5:
-; CHECK-NEXT:    [[PTR0:%.*]] = inttoptr i64 [[DATA]] to i32*
+; CHECK-NEXT:    [[PTR0:%.*]] = bitcast i8* [[DATADATA_PTR]] to i32*
 ; CHECK-NEXT:    tail call void @sink0(i32* [[PTR0]])
 ; CHECK-NEXT:    br label [[BB9:%.*]]
 ; CHECK:       bb7:
-; CHECK-NEXT:    [[PTR1:%.*]] = inttoptr i64 [[DATA]] to i32*
-; CHECK-NEXT:    tail call void @sink1(i32* [[PTR1]])
+; CHECK-NEXT:    [[PTR11:%.*]] = bitcast i8* [[DATADATA_PTR]] to i32*
+; CHECK-NEXT:    tail call void @sink1(i32* [[PTR11]])
 ; CHECK-NEXT:    br label [[BB9]]
 ; CHECK:       bb9:
 ; CHECK-NEXT:    ret void
 ;
 bb:
   %data = load i64, i64* %src, align 8
   br i1 %c0, label %bb3, label %bb7
 

 bb9:
   ret void
 }
 
 ; Negative test - all uses are casts, but non-identical ones.
 define void @n1(i1 zeroext %c0, i1 zeroext %c1, i64* nocapture readonly %src) {
 ; CHECK-LABEL: @n1(
 ; CHECK-NEXT:  bb:
-; CHECK-NEXT:    [[DATA:%.*]] = load i64, i64* [[SRC:%.*]], align 8
+; CHECK-NEXT:    [[TMP0:%.*]] = bitcast i64* [[SRC:%.*]] to i8**
+; CHECK-NEXT:    [[DATADATA_PTR:%.*]] = load i8*, i8** [[TMP0]], align 8
+; CHECK-NEXT:    [[PTR01:%.*]] = bitcast i8* [[DATADATA_PTR]] to i32*
 ; CHECK-NEXT:    br i1 [[C0:%.*]], label [[BB3:%.*]], label [[BB7:%.*]]
 ; CHECK:       bb3:
 ; CHECK-NEXT:    br i1 [[C1:%.*]], label [[BB4:%.*]], label [[BB5:%.*]]
 ; CHECK:       bb4:
 ; CHECK-NEXT:    tail call void @abort()
 ; CHECK-NEXT:    unreachable
 ; CHECK:       bb5:
-; CHECK-NEXT:    [[PTR0:%.*]] = inttoptr i64 [[DATA]] to i32*
-; CHECK-NEXT:    tail call void @sink0(i32* [[PTR0]])
+; CHECK-NEXT:    tail call void @sink0(i32* [[PTR01]])
 ; CHECK-NEXT:    br label [[BB9:%.*]]
 ; CHECK:       bb7:
-; CHECK-NEXT:    [[VEC:%.*]] = bitcast i64 [[DATA]] to <2 x i32>
+; CHECK-NEXT:    [[TMP1:%.*]] = ptrtoint i8* [[DATADATA_PTR]] to i64
+; CHECK-NEXT:    [[VEC:%.*]] = bitcast i64 [[TMP1]] to <2 x i32>
 ; CHECK-NEXT:    tail call void @sink2(<2 x i32> [[VEC]])
 ; CHECK-NEXT:    br label [[BB9]]
 ; CHECK:       bb9:
 ; CHECK-NEXT:    ret void
 ;
 bb:
   %data = load i64, i64* %src, align 8
   br i1 %c0, label %bb3, label %bb7

 bb9:
   ret void
 }
 
 ; Negative test - have non-cast users.
 define void @n2(i1 zeroext %c0, i1 zeroext %c1, i64* nocapture readonly %src) {
 ; CHECK-LABEL: @n2(
 ; CHECK-NEXT:  bb:
-; CHECK-NEXT:    [[DATA:%.*]] = load i64, i64* [[SRC:%.*]], align 8
+; CHECK-NEXT:    [[TMP0:%.*]] = bitcast i64* [[SRC:%.*]] to i8**
+; CHECK-NEXT:    [[DATADATA_PTR:%.*]] = load i8*, i8** [[TMP0]], align 8
+; CHECK-NEXT:    [[PTR01:%.*]] = bitcast i8* [[DATADATA_PTR]] to i32*
 ; CHECK-NEXT:    br i1 [[C0:%.*]], label [[BB3:%.*]], label [[BB7:%.*]]
 ; CHECK:       bb3:
 ; CHECK-NEXT:    br i1 [[C1:%.*]], label [[BB4:%.*]], label [[BB5:%.*]]
 ; CHECK:       bb4:
 ; CHECK-NEXT:    tail call void @abort()
 ; CHECK-NEXT:    unreachable
 ; CHECK:       bb5:
-; CHECK-NEXT:    [[PTR0:%.*]] = inttoptr i64 [[DATA]] to i32*
-; CHECK-NEXT:    tail call void @sink0(i32* [[PTR0]])
+; CHECK-NEXT:    tail call void @sink0(i32* [[PTR01]])
 ; CHECK-NEXT:    br label [[BB9:%.*]]
 ; CHECK:       bb7:
-; CHECK-NEXT:    tail call void @sink3(i64 [[DATA]])
+; CHECK-NEXT:    [[TMP1:%.*]] = ptrtoint i8* [[DATADATA_PTR]] to i64
+; CHECK-NEXT:    tail call void @sink3(i64 [[TMP1]])
 ; CHECK-NEXT:    br label [[BB9]]
 ; CHECK:       bb9:
 ; CHECK-NEXT:    ret void
 ;
 bb:
   %data = load i64, i64* %src, align 8
   br i1 %c0, label %bb3, label %bb7
 

llvm/test/Transforms/InstCombine/select.ll

 define i8* @test83(i1 %flag) {
 ; CHECK-LABEL: @test83(
 ; CHECK-NEXT:    [[X:%.*]] = alloca i8*, align 8
 ; CHECK-NEXT:    [[Y:%.*]] = alloca i8*, align 8
 ; CHECK-NEXT:    [[TMPCAST:%.*]] = bitcast i8** [[Y]] to i64*
 ; CHECK-NEXT:    [[X1:%.*]] = bitcast i8** [[X]] to i64*
 ; CHECK-NEXT:    call void @scribble_on_i64(i64* nonnull [[X1]])
 ; CHECK-NEXT:    call void @scribble_on_i64(i64* nonnull [[TMPCAST]])
-; CHECK-NEXT:    [[TMP:%.*]] = load i64, i64* [[X1]], align 8
-; CHECK-NEXT:    store i64 [[TMP]], i64* [[TMPCAST]], align 8
-; CHECK-NEXT:    [[V:%.*]] = inttoptr i64 [[TMP]] to i8*
-; CHECK-NEXT:    ret i8* [[V]]
+; CHECK-NEXT:    [[TMPTMP_PTR:%.*]] = load i8*, i8** [[X]], align 8
+; CHECK-NEXT:    [[TMP1:%.*]] = ptrtoint i8* [[TMPTMP_PTR]] to i64
+; CHECK-NEXT:    store i64 [[TMP1]], i64* [[TMPCAST]], align 8
+; CHECK-NEXT:    ret i8* [[TMPTMP_PTR]]
 ;
   %x = alloca i8*
   %y = alloca i64
   %x1 = bitcast i8** %x to i64*
   %y1 = bitcast i64* %y to i8**
   call void @scribble_on_i64(i64* %x1)
   call void @scribble_on_i64(i64* %y)
   %tmp = load i64, i64* %x1

llvm/test/Transforms/PhaseOrdering/instcombine-sroa-inttoptr.ll

bb:
   store i32* %i2, i32** %i3, align 8
   ret void
 }
 
 define dso_local i32* @_Z3foo1S(%0* byval(%0) align 8 %arg) {
 ; CHECK-LABEL: @_Z3foo1S(
 ; CHECK-NEXT:  bb:
 ; CHECK-NEXT:    [[I2:%.*]] = alloca [[TMP0:%.*]], align 8
-; CHECK-NEXT:    [[I1_SROA_0_0_I5_SROA_CAST:%.*]] = bitcast %0* [[ARG:%.*]] to i64*
-; CHECK-NEXT:    [[I1_SROA_0_0_COPYLOAD:%.*]] = load i64, i64* [[I1_SROA_0_0_I5_SROA_CAST]], align 8
+; CHECK-NEXT:    [[TMP0]] = bitcast %0* [[ARG:%.*]] to i8**
+; CHECK-NEXT:    [[I1_SROA_0_0_COPYLOADI1_SROA_0_0_COPYLOAD_PTR:%.*]] = load i8*, i8** [[TMP0]], align 8
+; CHECK-NEXT:    [[TMP1:%.*]] = bitcast i8* [[I1_SROA_0_0_COPYLOADI1_SROA_0_0_COPYLOAD_PTR]] to i32*
+; CHECK-NEXT:    [[TMP2:%.*]] = ptrtoint i8* [[I1_SROA_0_0_COPYLOADI1_SROA_0_0_COPYLOAD_PTR]] to i64
 ; CHECK-NEXT:    [[I_SROA_0_0_I6_SROA_CAST:%.*]] = bitcast %0* [[I2]] to i64*
-; CHECK-NEXT:    store i64 [[I1_SROA_0_0_COPYLOAD]], i64* [[I_SROA_0_0_I6_SROA_CAST]], align 8
+; CHECK-NEXT:    store i64 [[TMP2]], i64* [[I_SROA_0_0_I6_SROA_CAST]], align 8
 ; CHECK-NEXT:    tail call void @_Z7escape01S(%0* nonnull byval(%0) align 8 [[I2]])
-; CHECK-NEXT:    [[TMP0]] = inttoptr i64 [[I1_SROA_0_0_COPYLOAD]] to i32*
-; CHECK-NEXT:    ret i32* [[TMP0]]
+; CHECK-NEXT:    ret i32* [[TMP1]]
 ;
 bb:
   %i = alloca %0, align 8
   %i1 = alloca %0, align 8
   %i2 = alloca %0, align 8
   %i3 = bitcast %0* %i to i8*
   call void @llvm.lifetime.start.p0i8(i64 24, i8* %i3)
   %i4 = bitcast %0* %i1 to i8*

 
 declare dso_local void @_Z7escape01S(%0* byval(%0) align 8)
 
 declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
 
 define dso_local i32* @_Z3bar1S(%0* byval(%0) align 8 %arg) {
 ; CHECK-LABEL: @_Z3bar1S(
 ; CHECK-NEXT:  bb:
-; CHECK-NEXT:    [[I1_SROA_0_0_I4_SROA_CAST:%.*]] = bitcast %0* [[ARG:%.*]] to i64*
-; CHECK-NEXT:    [[I1_SROA_0_0_COPYLOAD:%.*]] = load i64, i64* [[I1_SROA_0_0_I4_SROA_CAST]], align 8
+; CHECK-NEXT:    [[TMP0:%.*]] = bitcast %0* [[ARG:%.*]] to i8**
+; CHECK-NEXT:    [[I1_SROA_0_0_COPYLOADI1_SROA_0_0_COPYLOAD_PTR:%.*]] = load i8*, i8** [[TMP0]], align 8
 ; CHECK-NEXT:    [[I5:%.*]] = tail call i32 @_Z4condv()
 ; CHECK-NEXT:    [[I6_NOT:%.*]] = icmp eq i32 [[I5]], 0
 ; CHECK-NEXT:    br i1 [[I6_NOT]], label [[BB10:%.*]], label [[BB7:%.*]]
 ; CHECK:       bb7:
+; CHECK-NEXT:    [[TMP1:%.*]] = bitcast i8* [[I1_SROA_0_0_COPYLOADI1_SROA_0_0_COPYLOAD_PTR]] to i32*
 ; CHECK-NEXT:    tail call void @_Z5sync0v()
-; CHECK-NEXT:    [[TMP0:%.*]] = inttoptr i64 [[I1_SROA_0_0_COPYLOAD]] to i32*
-; CHECK-NEXT:    tail call void @_Z7escape0Pi(i32* [[TMP0]])
+; CHECK-NEXT:    tail call void @_Z7escape0Pi(i32* [[TMP1]])
 ; CHECK-NEXT:    br label [[BB13:%.*]]
 ; CHECK:       bb10:
 ; CHECK-NEXT:    tail call void @_Z5sync1v()
-; CHECK-NEXT:    [[TMP1:%.*]] = inttoptr i64 [[I1_SROA_0_0_COPYLOAD]] to i32*
-; CHECK-NEXT:    tail call void @_Z7escape1Pi(i32* [[TMP1]])
+; CHECK-NEXT:    [[TMP2:%.*]] = bitcast i8* [[I1_SROA_0_0_COPYLOADI1_SROA_0_0_COPYLOAD_PTR]] to i32*
+; CHECK-NEXT:    tail call void @_Z7escape1Pi(i32* [[TMP2]])
 ; CHECK-NEXT:    br label [[BB13]]
 ; CHECK:       bb13:
-; CHECK-NEXT:    [[DOTPRE_PHI:%.*]] = phi i32* [ [[TMP1]], [[BB10]] ], [ [[TMP0]], [[BB7]] ]
+; CHECK-NEXT:    [[DOTPRE_PHI:%.*]] = phi i32* [ [[TMP2]], [[BB10]] ], [ [[TMP1]], [[BB7]] ]
 ; CHECK-NEXT:    ret i32* [[DOTPRE_PHI]]
 ;
 bb:
   %i = alloca %0, align 8
   %i1 = alloca %0, align 8
   %i2 = bitcast %0* %i to i8*
   call void @llvm.lifetime.start.p0i8(i64 24, i8* %i2)
   %i3 = bitcast %0* %i1 to i8*

llvm/utils/compare-stats.py

+#!/usr/bin/env python3
+'''
+Compare two .stats files generated by llvm tools
+
+compare-stats.py takes as argument two stats files to compare
+and output the result CSV on stdout
+
+Usage:
+  compare-stats.py total-old.stats total-new.stats
+'''
+
+import json
+import sys
+import collections
+
+result = {}
+
+def tassign(tup, index, val):
+    return tup[:index] + (val,) + tup[index + 1:]
+
+for arg in range(1, len(sys.argv)):
+  with open(sys.argv[arg], "r", encoding='utf-8',
+            errors='ignore') as f:
+    text = f.read()
+    try:
+      data = json.loads(text)
+    except:
+      print('ignored %s: failed to parse' % sys.argv[arg], file= sys.stderr)
+      continue
+    for key in data:
+      if not key in result:
+        result[key] = (0,0)
+      result[key] = tassign(result[key], arg-1, data[key])
+
+print("statistic name,baseline,proposed,Δ,%,|%|")
+for k,v in collections.OrderedDict(sorted(result.items())).items():
+  # Ignore non-changed stats
+  if v[0] == v[1]:
+    continue;
+  reldiff = v[1]-v[0]
+  absdiff = 0
+  absabsdiff = 0
+  if v[0]:
+    absdiff = reldiff/v[0]
+    absabsdiff = abs(absdiff)
+  print('{},{:d},{:d},{:+d},{:+.04f},{:.04f}'.format(k,v[0],v[1],reldiff,absdiff,absabsdiff))