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

Thu Mar 14 10:09:34 PDT 2019

Petar.Avramovic added a comment.

I agree that this is an important issue and that it should be implemented. I have already done some work in this direction and you can find a description of my work in the following text. Any comment is very welcome.

In short: I think that we should eliminate all copies and artifacts between two instructions in Legalizer. After such a change we should not need to look through copies and other artifacts.

My understanding of Instruction/Artifact

Instruction: 
Gets legalized, legalization does not depend on other instructions/artifacts. Instructions can be legalized in any order (currently are legalized bottom up for efficiency and use of stack data structure). During legalization Instruction can be legal (unchanged), or legalized.
Legalization can be done:
"in place" e.g. widen scalar. We mutate instruction and add few artifacts.
Replaced with a few new instructions and artifacts, original instruction gets deleted e.g narrow scalar.
New instructions and artifacts get pushed on stack where they wait to be legalized.

Artifact: Not an Instruction. 
Some properties: First we try to combine artifacts (during combine, artifact gets deleted and replaced with some instruction or with instruction+artifact). In case we fail to combine, Artifact becomes instruction and gets pushed into Instruction stack where it waits to get legalized.
Currently, there is only one chance for artifact to get combined.
It can happen that Artifact1 needs to wait another Artifact2 to become instruction and to get legalized producing new Artifact3. Artifact1 would want to combine with Artifact3.
It can not happen for Artifact4 to wait for an Instruction to produce Artifact5 during its legalization because we first legalize Instructions and then combine artifacts.

> The rationale is that the legalization may have been prevented because the legalization of the instruction couldn't cope with the artifacts in the middle.

This should be done for artifacts, if instruction is not legal, nothing can help it.
Generally :
If all uses of an Artifact that failed to combine are Instructions, then the Artifact should be turned into an Instruction.  
If one of the uses is an Artifact, then wait, maybe it could be combined later.

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

I would like to propose and push some changes to legalizer, and add few new opcodes to improve artifact combine. This changes should allow support for all types (that is for all big types that cannot fit in register and Opcodes that work with them should be narrow scalared, since small types are easier to handle). The descriptions of changes are given below.

(1) Instead of directly generating shifts/and when we combine G_SEXT/G_ZEXT with G_TRUNC we should generate instructions that represent explicit sign/zero extend in register (G_SEXT_EXPLICIT/G_ZEXT_EXPLICIT), and allow targets to legalize them.

(2) Add few new opcodes that represent implicit trunc of sign/zero(G_IMPLICIT_SIGN_TRUNC/G_IMPLICIT_ZERO_TRUNC). Could be used for function arguments that are implicitly sign/zero extended (in ll this is indicated by signext/zeroext), or for legalization of instructions. For example widen scalar of G_ICMP on mips32 could return s32 that is G_IMPLICIT_ZERO_TRUNC-ated to s1 since actual instruction that is going to be selected does implicit zero-extend to full register. This should improve generation, that is avoid generation, of G_SEXT_EXPLICIT/G_ZEXT_EXPLICIT when a simple copy would do.

(3) Copies that are in the way of some artifact legalization are products of some other artifact combines. We may want to generate G_COPY instead of COPY (e.g. when we combine G_ANYEXT and G_TRUNC), push them into instruction legalize stack, and eliminate them when they are about to be legalized. G_COPY because COPY is sometimes used for cross register class copy/move that gets expanded later. If Targets would not use COPY for such purposes we could get away with COPY, but just to be sure and allow targets to do what they want I would go for G_COPY. G_COPY should not be allowed to leave legalizer and can only be created in artifact combine. This change would address looking through copies and other opcodes.

(4) Combining Artifacts (as already described)

(5) Improve the elimination of dead instructions, as they are present at the moment. (This should get removed once we are sure that we do not leave dead instructions after artifact combines.)

After these changes we should be able to handle constants in much better way. For example we could figure out if constant should be sign or zero extended and generate proper value instead of currently always sign-extending. We could try to implement some constant elimination once they are properly legalized.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59339