[PATCH] D21791: [InstCombine] Fix for trunc folding build break

Sanjoy Das via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 28 10:57:38 PDT 2016


sanjoy added a comment.

Hi Anna,

In http://reviews.llvm.org/D21791#468817, @anna wrote:

>




> Could think of 3 possible ways to fix the build breakage caused by

>  type mismatch (the test case which showcases the breakage is

>  @trunc_avoid_bitcast):

> 

> 1. The RAUW  `return replaceInstUsesWith(CI, AvailableVal);` should be `return replaceInstUsesWith( CI, Builder->CreateBitOrPointerCast(AvailableVal, CI.getType(), CI.getName() + ".cast"));`

> 2. Avoid the RAUW when the `AvailableVal` type is not the same as `DestTy` of trunc.

> 3. Special case the trunc type in `FindAvailableLoadedValue` (i.e. check for exact match rather than `CastInst::isBitOrNoopPointerCastable(LI->getType(), AccessTy, DL)`) Error-prone solution whose benefit is minimal.

> 

>   I followed #2.

> 

>   What #1 does is replace the trunc with a bitcast or PointerCast. While generating bitcast would be useful if `FindAvailableLoadedValue` was used to replace loads, there does not seem to be a *strong* benefit in replacing the trunc with `bitcast` or `pointercast`. Looking at `SelectionDAG::visitBitCast` and all the pointer cast instructions, it may or may not be a noop. I didnot see an obvious benefit (i.e. hold true in all cases) in using `BitOrPointerCast` instead of `trunc`.


The normal LLVM meme is "bitcasts are free", so I'd be tempted to with
#1.  That is also less surprising (a "spurious" pointer cast won't
break optimization).  `ISDOpcodes.h` says that there are cases where
`ISD::BITCAST` will not be a no-op, but my guess is that it is still
better to replace a `trunc` with `bitcast` (i.e. even in the "worst"
case a `bitcast` will be no worse than a `trunc`).

> Also, this avoids possible performance regression creeping up due to

>  some specific target lowering or due to optimizations that benefit

>  `trunc` rather than `BitOrPointerCast`.


Given how ubiquitous `bitcast` s are in LLVM IR, I think that ^ is
unlikely (and if we find such an optimization that is broken by a
bitcast, we should fix the optimization).


Repository:
  rL LLVM

http://reviews.llvm.org/D21791





More information about the llvm-commits mailing list