[VPlan] Add getLiveInIRValue accessor to VPValue.

[fhahn]

Summary

This patch adds a new getLiveInIRValue accessor to VPValue, which
returns the underlying value, if the VPValue is defined outside of
VPlan. This is required to handle scalars in VPTransformState, which
requires dealing with scalars defined outside of VPlan.

We can simply check VPValue::Def to determine if the value is defined
inside a VPlan.

[fhahn]

ping

[fhahn]

ping :)

[Ayal]

This currently requires giving access to VPValue to those classes, so we
can call the protected constructor. But most of them can be removed in
the future, once the remaining single value def recipes inherit from
VPValue.

Such access seems unneeded as commented below; but in general may have been better to handle these remaining recipes first and avoid multiple befriendings, albeit temporary.

llvm/lib/Transforms/Vectorize/VPlanValue.h

64

As explained above, Def is null iff this VPValue is a "live-in", i.e., defined outside the loop (and used inside the loop).
So getOutOfScopeIRValue() (getLiveInValue()?) could simply return (getDef()? getUnderlyingValue() : nullptr), or its users could do so themselves directly?

[fhahn]

Thanks for taking a look!

In D92281#2479130, @Ayal wrote:

This currently requires giving access to VPValue to those classes, so we
can call the protected constructor. But most of them can be removed in
the future, once the remaining single value def recipes inherit from
VPValue.

Such access seems unneeded as commented below; but in general may have been better to handle these remaining recipes first and avoid multiple befriendings, albeit temporary.

Yes, originally this was not checking for VPDef, because when I posted the patch initially I did not want to block it on completing the transition. But that is done now, so just checking Def is much simpler.

I updated the code to just check VPDef (no need to befriend classes) and removed the function to getLiveInIRValue.

[fhahn]

llvm/lib/Transforms/Vectorize/VPlanValue.h

64

I did that and updated the function name. I think it is still worth to have it as a separate function, because there will be a few users in the following patches. Also, I think granting access to live-in IR values makes sense in general, even once we push back/reduce uses of getUnderlyingValue elsewhere.

[fhahn]

Add assert to make sure getLiveInIRValue is only called for 'live-in' VPValues.

[Ayal]

(how) are the VPPredInstPHIRecipe changes related to the introduction of getLiveInIRValue()?

Patch is (NFC).

[fhahn]

In D92281#2480441, @Ayal wrote:

(how) are the VPPredInstPHIRecipe changes related to the introduction of getLiveInIRValue()?

Patch is (NFC).

They are not directly related, i didn’t realise they were already there before undoing the changes that were part of the original patch.

I can split them off and land them separately

[Ayal]

ok, please land independent changes separately, thanks!

[fhahn]

Thanks Ayal! I landed the VPPredInstPHIRecipe changes separately in f73c09caa2a8.