[Mlir-commits] [mlir] [MLIR][LLVM] Support inrange in GEP (PR #220424)

[Letu Ren]

================
@@ -829,6 +829,11 @@ LogicalResult LLVM::GEPOp::verify() {
   if (getNoWrapFlags() == GEPNoWrapFlags::inboundsFlag)
     return emitOpError("'inbounds_flag' cannot be used directly.");
 
+  if (auto inrange = getInrangeAttr()) {
+    if (inrange.getLower().sge(inrange.getUpper()))
+      return emitOpError("expected 'inrange' end to be larger than start");
----------------
FantasqueX wrote:

Short summary:
1. In LLVM, a constant GEP and an instruction GEP are different types while in mlir, there are only one type. So, in LLVM, verifying all operands is not an issue. Because constant GEP type already restricts it.
2. I don't think pattern matching is a good method because a constant GEP is also a constant expression. We must verify recursively which is not cheap.

I also draft a respond using LLM which is more complete than reasons above.

LLVM does not check this because a constant GEP and an instruction GEP are different types. `GetElementPtrConstantExpr` stores only `Constant *` operands, and `inrange` exists only on that class. `GetElementPtrInst` takes ordinary `Value *` operands and has no `inrange` field. A nested GEP is already a Constant operand of the outer GEP, so there is nothing to prove.
MLIR has one `llvm.getelementptr`. Its base and indices are SSA values, so "this is an LLVM constant expression" is not a property of the operation.
`m_ConstantInt` does not make that check slightly stricter. It rejects GEPs that are already constant expressions. The base may be `llvm.mlir.addressof` or another `llvm.getelementptr`, and an index may be a constant ptrtoint or add that does not fold to an integer. Those lower to `llvm::Constant` and export successfully. Rejecting them is a false positive, not a rule that only forbids computations which might fold during export.
Accepting those cases means walking the SSA definitions and reimplementing which LLVM constant expressions are allowed. That is the walk LLVM avoids by using two classes. Translation already checks `isa<llvm::Constant>` after lowering, which matches LLVM's rule.

https://github.com/llvm/llvm-project/pull/220424