[PATCH] D50465: [InstCombine] Optimize redundant 'signed truncation check pattern'.

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

LGTM.

About the autogen script problem - we could hack that to use something less likely than 'TMP' as its regex string (eg, 'AUTOGEN'), but that will result in significant test file churn if someone updates an existing file.



================
Comment at: lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1311-1316
+  assert(I.getOpcode() == Instruction::And);
+  // We are looking for an 'and' of two icmp's
+  auto *ICmp0 = dyn_cast<ICmpInst>(I.getOperand(0));
+  auto *ICmp1 = dyn_cast<ICmpInst>(I.getOperand(1));
+  if (!(ICmp0 && ICmp1))
+    return nullptr;
----------------
lebedev.ri wrote:
> spatel wrote:
> > Can this be moved within InstCombiner::foldAndOfICmps() to eliminate these preliminary checks?
> > 
> > Any chance for shared functionality with foldAndOrOfICmpsOfAndWithPow2() or simplifyRangeCheck()?
> I don't think i should squeeze it into `InstCombiner::foldAndOrOfICmpsOfAndWithPow2()`,
> as that looks for a *very* specific, different pattern.
> 
> `InstCombiner::simplifyRangeCheck()` also does not look too promising,
> we will then only handle the extreme cases (which is, admittedly, probably
> sufficient for my needs right now), but not the general case :/
> 
> None of those handle this case because they mostly require for the both icmp's
> to operate on the same value, and don't understand what
> ```
> %t = add        i32 %arg,    128
> %r = icmp   ult i32 %t,      256
> ```
> (and similar patterns) mean.
> 
> So i have sunk it down into `InstCombiner::foldAndOfICmps()` for now..
Ok, thanks for checking. I think we've noted this before - the whole thing is a complicated mess, and there should be some better way to generalize it, but it's not clear how to do it.


Repository:
  rL LLVM

https://reviews.llvm.org/D50465