[PATCH] D138637: [InstCombine] Combine opaque pointer single index GEP and with src GEP which has result of array type

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:2084
+  // GEP having single index.
+  if (PtrTy->isOpaquePointerTy() && Src->getResultElementType()->isArrayTy() &&
+      GEP.getNumIndices() == 1) {
----------------
Do we need an explicit `PtrTy->isOpaquePointerTy()` check?
Do we need an one-use check on the inner (`Src`) gep?
Do we want to perform the same LICM-appeasing checks about their loop disposition?


================
Comment at: llvm/test/Transforms/InstCombine/opaque-ptr-gep-of-gep.ll:1
+; RUN: opt -S -instcombine -opaque-pointers < %s | FileCheck %s
+
----------------
Is explicit `-opaque-pointers` needed?
Just write the test to not have typed pointers to begin with.


================
Comment at: llvm/test/Transforms/InstCombine/opaque-ptr-gep-of-gep.ll:6
+
+define void @gepofgep1(i64 %in1, i64 %in2, ptr %out) {
+; CHECK-LABEL: @gepofgep1(
----------------
It isn't obvious how these tests differ from each other.
They could use a comments.


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

https://reviews.llvm.org/D138637