Redundant ptrtoint/inttoptr instructions

Hi all,

We noticed a lot of unnecessary ptrtoint instructions that stand in way of some of our optimizations; the code pattern looks like this:

bb1:
%int1 = ptrtoint %struct.s* %ptr1 to i64

bb2:
%int2 = ptrtoint %struct.s* %ptr2 to i64

%bb3:
%phi.node = phi i64 [ %int1, %bb1 ], [%int2, %bb2 ]
%ptr = inttoptr i64 %phi.node to %struct.s*

In short, the pattern above arises due to:

  1. InstCombine introducing a bitcast while performing a canonicalization in combineLoadToOperationType() [1]

  2. GVN performing “load coercion” and replacing a load with a ptrtoint (ptrtoint is due to the bitcast)

  3. SROA replacing store- and load-instructions with phi nodes.

The question is: is it a good idea to clean ptrtoint/inttoptr instructions inside SROA or should it be done higher in the pass pipeline? More details below.

The canonicalization in instruction combining is the root cause of the problem, more information about the actual transformation and motivation behind it can be found in this RFC [2]. Based on our experiments, getting rid of the canonicalization reduces the number of ptrtoint instructions by 16% and inttoptr instructions by 25%. However, it seems that overall, LLVM benefits from the canonicalization and, at least back in 2015, many people supported adding it to LLVM. So, I would like to find a way to keep it while removing the ptrtoint-s that are unnecessary, and doing clean-up in SROA is a straightforward way to achieve that.

What does everybody think?

Thanks,
Alexey

[1] combineLoadToOperationType() https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp#L556
[2] [LLVMdev] RFC: Missing canonicalization in LLVM http://lists.llvm.org/pipermail/llvm-dev/2015-January/080956.html

Optimizing int2ptr + ptr2int (or vice versa) is tricky business.

I would prefer it to be outside SROA, I think.

We should be very careful and restrictive about it.

My initial thought is that the pattern below can be optimized

but I haven’t spend too much time on it.

~ Johannes

My general feeling is this: No optimizations should be creating int2ptr/ptr2int. We really need to fix them all. They should use pointer casts and i8* GEPs. This has, unfortunately, been a problem for a long time. As Johannes says, optimizing int2ptr/ptr2int is very tricky. In part, becaue all dependencies, including implicit control dependencies, end up being part of the resulting aliasing based-on chain, and we have a set of bug reports discussing this is great detail.

-Hal

Unfortunately, inttoptr cannot be optimized away since it has an
effect on pointer aliasing semantis. See for instance the discussion
at https://bugs.llvm.org/show_bug.cgi?id=34548

Michael

My general feeling is this: No optimizations should be creating int2ptr/ptr2int. We really need to fix them all. They should use pointer casts and i8* GEPs. This has, unfortunately, been a problem for a long time. As Johannes says, optimizing int2ptr/ptr2int is very tricky. In part, becaue all dependencies, including implicit control dependencies, end up being part of the resulting aliasing based-on chain, and we have a set of bug reports discussing this is great detail.

See, for example, https://bugs.llvm.org/show_bug.cgi?id=34548

-Hal

We recently started introducing some more of these to represent no-op address space conversions in cases where pointer bits are reinterpreted. There’s currently a whole in the IR where there’s no other way to represent a pointer reinterpret. To fix this case, I think we need to re-allow bitcasts between pointers with different address spaces, but the same size.

-Matt

Makes sense to me.

-Hal