This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] Simplify chain of GEP with constant indices
AbandonedPublic

Authored by huangjd on May 5 2022, 8:14 PM.

Details

Summary

Detects a chain of GEP where the first and last instructions
have constant indices, and combines them into one if such transformation
is valid

Diff Detail

Event Timeline

huangjd created this revision.May 5 2022, 8:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 5 2022, 8:14 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
huangjd requested review of this revision.May 5 2022, 8:14 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 5 2022, 8:14 PM
huangjd updated this revision to Diff 427523.May 5 2022, 8:26 PM

[InstCombine] Simplify chain of GEP with constant indices

SUMMARY:
Detects a chain of GEP where the first and last instructions
have constant indices, and combines them into one if such transformation
is valid

reviewers: davidxl, Carrot, nikic, spatel, reames

nikic requested changes to this revision.May 6 2022, 2:02 AM

This is a good idea, but I think the implementation approach here is too complicated and fragile. This can be handled as a combination of two transforms that are both simpler and more general:

  1. Reassociate constant GEPs, either by transforming gep (gep %p, C), %x to gep (gep %p, %x), C, or the other way around. This kind of reassociation is generally useful as a canonicalization even if nothing folds for redundancy elimination reasons. (It would also be possibly to reassociate in the other direction, but I think for GEPs we prefer this one, as constant offset addressing modes are pretty much universally supported.)
  1. Combine constant GEPs. We already do this, but sometimes fail based on element type mismatch. This can be fixed either by canonicalizing all constant GEPs to i8 (probably better) or by doing so only when it allows combining GEPs that otherwise couldn't be combined.
This revision now requires changes to proceed.May 6 2022, 2:02 AM

Perhaps split out part of the patch that handles the following and then work on a patch to do reassociation (nikic's comment)

; result = (struct.C*) p + 3
define %struct.C* @offsetDivisible(i64* %p) {
; CHECK-LABEL: @offsetDivisible(
; CHECK-NEXT: [[TMP1:%.*]] = getelementptr inbounds i64, ptr [[P:%.*]], i64 3
; CHECK-NEXT: [[TMP2:%.*]] = getelementptr inbounds [[STRUCT_C:%.*]], ptr [[TMP1]], i64 1
; CHECK-NEXT: ret ptr [[TMP2]]
;

%1 = getelementptr inbounds i64, i64* %p, i64 3
%2 = getelementptr inbounds %struct.C, %struct.C* %1, i64 1
ret %struct.C* %2

}

@nikic

  1. Combine constant GEPs. We already do this, but sometimes fail based on element type mismatch. This can be fixed either by canonicalizing all constant GEPs to i8 (probably better) or by doing so only when it allows combining GEPs that otherwise couldn't be combined.

Where are the tests located for combining constant GEP? I think currently it only work with one direction (merging the second one into the first), so I will need to make it work both ways.

nikic added a comment.May 11 2022, 1:31 AM

@nikic

  1. Combine constant GEPs. We already do this, but sometimes fail based on element type mismatch. This can be fixed either by canonicalizing all constant GEPs to i8 (probably better) or by doing so only when it allows combining GEPs that otherwise couldn't be combined.

Where are the tests located for combining constant GEP? I think currently it only work with one direction (merging the second one into the first), so I will need to make it work both ways.

There are some tests starting here:; https://github.com/llvm/llvm-project/blob/34b6f206cbab8471abf29739dab981bd8b868a65/llvm/test/Transforms/InstCombine/opaque-ptr.ll#L172 The code for this is in https://github.com/llvm/llvm-project/blob/34b6f206cbab8471abf29739dab981bd8b868a65/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp#L2025.

Though as already mentioned, the most general fix for this is to canonicalize all constant GEPs to use i8 element type, which ensures that two constant GEPs can always be combined.

What about merging non-opaque constant indexed GEP tests?

huangjd abandoned this revision.May 17 2022, 6:54 PM