LGTM.

LGTM.

LGTM.

Algorithm is similar to the one SDAG uses in SelectionDAGLegalize::ExpandLegalINT_TO_FP
The difference is that here we use BuildPair instead of sequence of two i32 stores followed by f64 load since it results in one instruction less.

LGTM. Let's wait for @sdardis.

Oh, I see. Then we have to undo combines here. Let's what for Simon.

This patch targets very specific values(1,-1) but there are more. The trick for D62341 is that msa vector add/sub imm accept 5 bit unsigned imm, so it is ok to switch from add imm to sub imm (also sub to add) if it changes imm form negative to positive. I think it is cleanest to have some hook and ask target if it would like to transform sub into add (add to sub) with imm.

LGTM.

One nit, inlined, other then that looks good.

LGTM.

LGTM.

Split changes to three smaller patches.

@aemerson @arsenm
test-suite/SingleSource/UnitTests/2003-05-31-LongShifts.c used to give bad result in high bits for some values,
changes to narrow scalar for G_SHL, G_ASHR and G_LSHR fixes that on MIPS32. Does it work for you?

I think it’s okay not to combine legal artifacts

Often they get combined before even asking if they are legal or not, there is no strict agreement on what should be combined or not it's kind of random and depends on the order we attempt combines.

Therefore, I don’t think adding more combines would improve the legalizer. We should probably work on the current legalization strategy first before doing something like that. What do you think?

I see two routes atm, one is this patch and no more combines, other is D65894 as it deals with combining non-legal artifacts, and to catch combines of legal artifacts we could add more combines, like D62338 and for example add combine for G_MERGE, similar to G_TRUNC, by inspecting uses of G_MERGE def operands in hope to find G_UNMERGE and perform combine from that G_UNMERGE. With such combines it is possible to visit def before all it uses have been visited and tracking of dead instructions becomes a little difficult. It all comes down to how we menage Observer lists that hold instructions/artifacts.
If we want to handle everything I think it is best to catch Artifact that failed to combine and retry or legalize under some conditions, the conditions when to retry combine or legalize depend on set of available combines(and maybe it would have to be reduced to set of more basic combines).

In the mean time, I would like to commit D65894 if you don’t have any concerns about it. It will allow us to legalize the instructions blocked by illegal artifacts without trying to improve the generated code.

Sure, let's go with D65894 for start.

Removed capture by value in lambda.

Removed capture by value in lambda.

In getAddrNonPICSym64 Relocation looks to come from another SDValue (Hi bellow)
MipsISD::Hi doesn't seem to be connected with any of the target flags (like MipsII::MO_ABS_HI)

SDValue Shift = DAG.getNode(ISD::SHL ...

@sdardis What is the difference between MipsHi and MipsLo SD nodes? Some of their patterns are the same, e.g. :

def : MipsPat<(add GPR64:$hi, (MipsHi (i64 tglobaladdr:$lo))),
              (DADDiu GPR64:$hi, tglobaladdr:$lo)>, ISA_MIPS3, GPR_64, SYM_64;

I am not really sure if this is correct way to fix PR42736. I looked in td files and found a few inconsistencies, again I might have misinterpreted something.

I don't think that matters as long as you process the rest of the instructions before aborting. That's what D65894 does, it keeps processing the instructions in InstList, then tries to combine the illegal artifacts if there is anything new. Do you have a test case that requires more complicated logic?

Yes, D65894 works perfectly fine for non legal artifacts, but it can't deal with legal artifacts that could be combined.
File withD61787.txt in D65199
line 920 in second file, Inspecting Artifact : %10:_(s32), %11:_(s32) = G_UNMERGE_VALUES %6:_(s64)

In this case, moving %1 to AuxiliaryArtifactList doesn't make any difference because combining %9 will remove it anyway, even if it was in InstList. Since you already check the source registers before legalizing the artifacts, this shouldn't be a problem. What do you think?

Well in this example it is not a problem, but in more complicated case:
moving %1 to AuxiliaryArtifactList means that %1 will be legalized first if %9 failed to combine it away, %9 might be able to combine away the artifact that was produced during legalization of %1.
In general, this patch only combines Artfact from ArtifactList with some auxiliary artifact from AuxiliaryArtifactList, if something gets legalized to legal it will not dispersal from MF during some combine. It's a bit confusing to see instruction said to be legal during legalization but it is not in the final MF.

@volkan Regarding D65894.
Here, artifacts that failed to combine are moved to RetryList and we retry to combine them. Once all of the MachineInstrs that define use operands of our artifact are processed (are not in any of the Observer Lists) we turn artifact into an instruction. This way we have more opportunities for combines. e.g. in /test/AMDGPU/GlobalISel/legalize-unmerge-values.mir , this patch catches G_MERGE/G_UNMERGE combine that D65894 cannot catch because those two are declared legal.
As for the other test changes in D65894, all of them are essentially here.
I also tried test/CodeGen/AArch64/GlobalISel/retry-artifact-combine.mir and got similar output, but with one less copy instr.
there are differences in test/CodeGen/AMDGPU/GlobalISel/legalize-{xor|and|or}.mir
D65894 managed to combine few more G_TRUNC + G_ANYEXT but in the end both patches crash on %0:_(<4 x s8>) = G_TRUNC %29:_(<4 x s32>), because order of combine attempts is different this patch crashed before attempting to combine mentioned G_TRUNC + G_ANYEXT.
Please check if I missed something the this patch does not cover compared to D65894.

Refresh test changes. Ping.