[PATCH] D59227: [GlobalISel][Utils] Teach getConstantVRegVal how to look through trunc and z|sext

[Quentin Colombet via llvm-commits]

> On Mar 13, 2019, at 11:08 AM, Matt Arsenault via Phabricator via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> arsenm added a comment.
> 
> In D59227#1427702 <https://reviews.llvm.org/D59227#1427702>, @qcolombet wrote:
> 
>> 
> 
> 
> 
> 
>> I agree that the G_CONSTANT are a mov to a legal result register. However, I don't think they should be legalized on there own. Essentially, if we truncate/extend the constants directly during the legalization of their uses, they naturally become legal (the illegal ones become dead). I.e., we don't have to handle G_CONSTANT explicitly. This is true only if the builders don't create thing like ext(cst), like they do today, because then we indeed have to legalize those immediate moves.
> 
> I'm not sure why we would want special rules for constant handling.

Again that’s just my personal opinion, but my point is, because we don’t have to do anything with them when everything else do the right(TM) thing :).

> I think it would be cleaner to just keep them treated as any other normal instruction.

It’s because they are treated like normal instruction that we need this patch right now.

> I'm assuming you mean the legalizer by builder.

No I really meant builder. Admittedly we would require to do the constant folding for exts and truncs only in the legalizer, but at this point there isn’t much incentive to not be consistent with every pass.

> If you're going to try to avoid the intermediate legalization artifacts by looking at the uses, I don't see why you wouldn't change the legalizer worklist to do this for every instruction.

That’s a fair point.
I didn’t see it this way because ext(cst) are **not** simplified as part of legalization artifact, but produce simplifiable legalization artifacts after we legalize the constant ext(cst) => ext(truncates(cst)).
That said, I am not in favor of any complicated scheme regarding how we handle the legalizer wordlist, but this makes me think that another way of looking at the problem is maybe we shouldn’t fail legalization before we are actually stuck.
Let me illustrate on an example.

Let say we ask for the promotion of UREM for non-power-of-2 in:
%cst = G_CONSTANT i14 cst
= UREM i14 …, %cst

We will get:
%cst = G_CONSTANT i14 cst
%promotedCst = G_ZEXT i14 %cst to i16
= UREM i16 …, %promotedCst

Then, we ask for lowering of UREM for i16:
%cst = G_CONSTANT i14 cst
%promotedCst = G_ZEXT i14 %cst to i16
= UDIV i16 …, %promotedCst
…

Now, the problem is we need to be able to legalize UDIV within this legalization step, I’ll come back to that, and let say we can only do that when RHS is a constant. So at this point, we have one of the following option:
1. Look through extensions (what this patch does)
2. Lower to a lib call (would need to be undone assuming the target supports this lib call at all)
3. Have promotedCst being an actual constant instead fo a zext of another one (which is not possible since we legalize constant and directly creating another constant may actually produce an illegal one and we’ll end up with an infinite loop)

Let say that we allow the legalization step to fail (unlike today) and requeue the instruction for later legalization. As long as we make progress elsewhere we keep going. Then the artifacts in the way of the constant would eventually be removed and we fixed the problem.

That actually sounds like a better solution and this is orthogonal to the way we handle constant.

What do you think?

> 
> 
> Repository:
>  rL LLVM
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D59227/new/
> 
> https://reviews.llvm.org/D59227
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits