[PATCH] D78353: [LV] Don't emit the Vector IV if it is not used.

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer marked an inline comment as done.
SjoerdMeijer added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:1920
   Value *ScalarIV = CreateScalarIV(Step);
   CreateSplatIV(ScalarIV, Step);
   buildScalarSteps(ScalarIV, Step, EntryVal, ID);
----------------
Ayal wrote:
> Would it be possible to avoid generating the undesired broadcasting by wrapping this CreateSplatIV() under an "if (needsVectorInduction(EntryVal))" that checks if any user !shouldScalarizeInstruction(), analogous to needsScalarInduction(EntryVal) above?
> 
> Better avoid generating and carrying potential garbage to begin with if it's easy; or potentially augment cse() clean-up at the end.
> Would it be possible to avoid generating the undesired broadcasting by wrapping this CreateSplatIV() under an "if (needsVectorInduction(EntryVal))" that checks if any user !shouldScalarizeInstruction(), analogous to needsScalarInduction(EntryVal) above?

I agree that this would be nicer, but it looks like  `needsScalarInduction` is an easier case as it simply looks at orginal loop instructions and see if they need to be scalarised. For the possibly new `needsVectorInduction` variant, I think we would need to look at the uses of the vector IV in the vector code, which hasn't been created yet. And since we need to look at the uses, which is what this patch is doing, I thought doing this afterwards in the fixup routine was a simple and reasonable compromise. For example, I see cases of masked/load stores using the vector IV, and I don't think I have all the information at this point to check all uses. 

> Better avoid generating and carrying potential garbage to begin with if it's easy; or potentially augment cse() clean-up at the end.

As I wrote above, I am not sure if it is easier or even possible to avoid generating the dead code. But I will have a look again to see if I haven't missed anything.  If this is not possible, I understand your preference is to have some form of DCE running here?  I thought about that, but wasn't sure if that would be overkill for just removing the vectorIV...


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

https://reviews.llvm.org/D78353