[PATCH] D22001: [DSE] Remove dead stores in end blocks containing fence

[Anna Thomas via llvm-commits]

That’s right Duncan. The implicit conversion is not present when we take the reference version of isa: 

isa<FenceInst>(*BBI)

Was thinking of the pointer version. 

So, I’ve changed it to the above instead of isa<FenceInst>(BBI)

Anna
> On Jul 7, 2016, at 4:28 PM, Anna Thomas <anna at azul.com> wrote:
> 
> anna marked 2 inline comments as done.
> anna added a comment.
> 
> In http://reviews.llvm.org/D22001#477281, @dexonsmith wrote:
> 
>> I still think this should be the version of `isa<>` that takes a reference.
> 
> 
> Yes, I've changed it to directly take the reference (as done in other dyn_cast checks such as `LoadInst`, `MemTransferInst`  just below the fenceInst check).
> 
>  if (isa<FenceInst>(BBI))
>        continue;
> 
> It's better to have it as a reference, as the same is done in all other checks for `BBI`.
> 
> The implicit conversion cost as stated in the lang ref:
> Unfortunately, these implicit conversions come at a cost; they prevent these iterators from conforming to standard iterator conventions, and thus from being usable with standard algorithms and containers. For example, they prevent the following code, where B is a BasicBlock, from compiling:
> 
> llvm::SmallVector<llvm::Instruction *, 16>(B->begin(), B->end());
> Because of this, these implicit conversions may be removed some day, and operator* changed to return a pointer instead of a reference.
> 
> 
> http://reviews.llvm.org/D22001
> 
> 
>