[PATCH] D36679: [InstCombine] Added support for: trunc(ashr(mul(sext(...), sext(...))) -> ashr(mul(...))

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:468
     if (match(I->getOperand(1), m_APInt(SA))) {
-      uint64_t ShiftAmt = SA->getLimitedValue(BitWidth-1);
+      uint32_t ShiftAmt = SA->getLimitedValue(BitWidth-1);
 
----------------
aaboud wrote:
> spatel wrote:
> > Why change this in this patch? Seem like that might be better as just 'unsigned', but either way, I think it's an independent diff.
> I change it to uint32_t to be consistent with line 507 (the case for AShr).
> Sure, we can commit this separately.
> Do we need a review for this?
> Do you want to take care of it? or I should just commit it now?
Ah, I see the LangRef says we're allowed to go up to 24 bits of width:
http://llvm.org/docs/LangRef.html#integer-type

So I guess it's ok to specify 32-bit here...or just change them all to 'unsigned'? Either way seems fine to me. Feel free to commit separately.


https://reviews.llvm.org/D36679