[PATCH] D60413: [BDCE] SExt -> ZExt when no sign bits is used and instruction has multiple uses

Tue Aug 18 09:12:21 PDT 2020

bjope added a comment.

In D60413#2223875 <https://reviews.llvm.org/D60413#2223875>, @lebedev.ri wrote:

> In D60413#2223866 <https://reviews.llvm.org/D60413#2223866>, @bjope wrote:
>
>> Having some problems with this patch downstream:
>>
>> 1. Patch does not update any dbg.value that might refer to the value that now is zext:ed instead of sext:ed. So debugging experience might become weird. It might be possible to rewrite debug uses, using some nifty debug expression to express  a trunc+sext of the value. But in my case it was an <128 x i40> vector, and we got no dwarf expressions to express a vector sext afaik. Instead I guess any debug uses should be invalidated.
>
> Isn't that an issue for any of the other places that do such a transform, among other things?
> This very transform also exists in InstCombine and CVP.

That might be a bigger problem ofcourse. But if a transform doing RAUW isn't replacing with the same thing, then I guess it need to salvage/undef the debug uses and only replace the non-debug uses. Otherwise I think the debug values will be incorrect.

>> 2. My target really prefers sext before zext when it comes to extending from <128 x i32> to <128 x i40>. Well, at the moment the zext isn't even legal. So now I get compilation failures. Pretty hard to redo the (global) demanded bits analysis during ISel to perform the reverse transform. Not sure really how such problems are dealt with in general. For my use case it had been much better to hoist the trunc that implies that the upper bits is irrelevant, and that way avoiding an <128 x i40> vector with irrelevant upper bits altogether.
>
> You can always just do `sext` and then mask unneeded high bits away.
>
>> At least (1) is something that I think we need to do something about.
>> When it comes to (2) I'd welcome any suggestions on how I could avoid this transform for my target (or ideas about how to add a reverse transform closer to codegen).

Oh, I wish I had a vector-AND. I can probably implement the zext using SHL+LSHR, but that is two operations instead of one (SEXT+AND would also be two ops instead of one single SEXT). The target also support sextload but not zextload, so I might miss out on that as well. In general the target got better support for sext than zext (also in some scalar cases). Sext is often for free, while zext isn't. Maybe I need to teach CodegenPrepare to turn zext into sext if the upper bits are irrelevant. Undoing all these transforms changing zext into sext.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60413