[PATCH] D65380: [InstCombine] Shift amount reassociation: shl-trunc-shl pattern

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/IR/Constants.cpp:1590-1594
+Constant *ConstantExpr::getZExtOrSelf(Constant *C, Type *Ty) {
+  if (C->getType() == Ty)
+    return C;
+  return getZExt(C, Ty);
+}
----------------
spatel wrote:
> Would we better off copying the codegen sibling API (same type implicitly returns self)?
> 
> SelectionDAG::getZExtOrTrunc()
I'm not sure what you have in mind here, `ConstantExpr::getZExt()`
will (rightfully) assert if zext is unsuitable there.
`ConstantExpr::getBitCast()` won't, so i suppose i could just use it,
but i really don't need bitcast here, i really want either zext or nothing.

Please can you be more specific as to what you envision here?


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp:39
+
+  // There may be a truncation before the next shift; take note and ignore it.
   Instruction *Sh1;
----------------
spatel wrote:
> That comment isn't clear to me.
>   "If there is truncation of the shifted value, we must 
>   adjust the constraints of this fold and recreate that op." ?
How about:
```
If there is a truncation inbetween these two shifts,
we must make note of it, and look through it.
The truncation will impose additional legality checks
on the transform.
```
?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65380