[PATCH] D59339: [GISel][Legalizer][WIP][RFC] Retry legalization for failing instrs when artifacts are around

Thu Mar 14 10:27:55 PDT 2019

qcolombet added a comment.

Hi Petr,

> Instructions:
>  Gets legalized, legalization does not depend on other instructions/artifacts.

That's unfortunately not true. For instance, for some out-of-tree target, we can legalize DIV by constant and that's all.
Remember that the legalization process can introduce target specific instruction, thus, it is entirely possible that an instruction is legal only in a given sequence of instruction. What this means is that it will likely be legalized directly into a pseudo or an actual instruction, in other words, something target specific.

> This should be done for artifacts, if instruction is not legal, nothing can help it.

I don't get that part.
To go back to my example, I cannot legalize my DIV until I see the constant, however the constant may be hidden behind artifacts and it will become legal only when they are removed.
E.g.,

  DIV ... (ext(trunc cst)) => illegal
  DIV ... (cst) => legal

> If all uses of an Artifact that failed to combine are Instructions, then the Artifact should be turned into an Instruction.

Agree.

> If one of the uses is an Artifact, then wait, maybe it could be combined later.

Do you see cases where we indeed have to wait in the current scheme where we deal with every instruction before looking at cleaning at the artifacts?
Basically, do you have an example?

> Only G_CONSTANTS have to be handled differently (although G_CONSTANTS are formally Instructions now, this should be changed to allow combining).

After my conversation with @arsenm I have mixed feeling about that. On one hand we could treat G_CONSTANT differently because it seems more efficient to me, on the other hands, if we deal with artifacts properly, we don't have to deal with them differently and I like the consistence here.

#1 and #2 sounds sensible to me.

#3 seems unnecessary in the sense that I am pretty sure we could just not insert the copy in the first place.

I am not sure I see the need for changing #4. This goes back to my question of do you have an example?

#5 somewhat agree, though they are not really in the way of anything right now.

> We could try to implement some constant elimination once they are properly legalized.

Don't we have that for free already? If something is not dead we keep it, otherwise we remove it. What am I missing?

Cheers,
-Quentin

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59339