LGTM

LGTM

LGTM

In D37817#876063, @arsenm wrote:

In any case independent of sub register questions, I think this would be better off done in the existing pass by adding variants with tied operands. This is how I am handling this problem currently in D38070/D38071 for mad_mix

LGTM

LGTM

LGTM

Rebase to master.

Added comment.

Extracted struct from the getPostRAMutations().

LGTM

In D37887#874537, @arsenm wrote:

In D37887#874523, @rampitec wrote:

Can you please restore foo_used and just fix calling convention? This case is not covered now.

That's the new func_used I added, which is the same thing.

Can you please restore foo_used and just fix calling convention? This case is not covered now.

LGTM

It is not redundant. It is here to get rid of unused library functions before we start optimizing them.

Removed MaxBB limit.

LGTM

LGTM

LGTM

LGTM

Please add a comment to the source why it was needed.

In D37817#869898, @arsenm wrote:

I think we need to decide an overall strategy for dealing with instructions that only partially update the registers. GFX9 really complicated this issue by changing new instructions to preserve the high bits, and adding a control bit to some instructions to change the high bit behavior.

I started dealing with this to add the d16 loads and stores. We can still do this SSA by adding variants of the instructions with tied operands that preserve one half of the instructions, which would probably be less painful than adding another post-SSA pass that needs to deal with liveness. One issue is we still get suboptimal regalloc in some cases, so I'm debating adding new 16-bit subregister classes so subrange liveness tracking works.

LGTM

Changed to let shouldClusterMemOps decide as discussed.

In D37698#870072, @MatzeB wrote:

In D37698#869935, @rampitec wrote:

As an alternative I can just drop this portion of code from BaseMemOpClusterMutation::clusterNeighboringMemOps() completely:

if (MemOpRecords[Idx].BaseReg != MemOpRecords[Idx+1].BaseReg) {
  ClusterLength = 1;
  continue;
}

and let TII->shouldClusterMemOps() decide. Maybe it is better. Thoughts?

Indeed, that sounds like the best/easiest solution so far!

In D37698#869839, @MatzeB wrote:

• When seeing the name doMemOpsHaveSameBasePtr I would expect this to do exactly BaseReg1 == BaseReg2. Can you explain how you happen to have cases with different base pointers that still reference the same object?

In D37755#869341, @hfinkel wrote:

Please see my comments in D37698. If the primary purpose of this on AArch64 is to enable the AArch64LoadStoreOptimizer, I'm not sure that this makes sense.

In D37698#868147, @vpykhtin wrote:

SIInstrInfo::doMemOpsHaveSameBasePtr looks generic enough to be default indeed but should we commit this first and then make doMemOpsHaveSameBasePtr default so we could rollback to this one in case of severe regressions? LGTM by the way.

In D36849#866812, @tstellar wrote:

There should be an explanation of what this pass does and why it is better than LLVM's default inliner and some benchmark data showing which applications / games this helps.

Added file brief and more comments for thresholds.

LGTM

Updated comment as suggested.

Renamed callback as suggested by Brian.
Cleanup the test.

In D37389#866644, @arsenm wrote:

LGTM with the minor cleanup

LGTM

Added f16 case.

In D37595#864031, @arsenm wrote:

In D37595#864026, @rampitec wrote:

It shall be caught by the operands analysis below and added only if needed. Do you have test showing problem?

Yes, it's pretty big though. It seems to only remove one s_or_b64 in it though.

It shall be caught by the operands analysis below and added only if needed. Do you have test showing problem?

In fact this is all not needed. We just cannot produce such instructions and the testcase was artificial with the bogus td file change.

Actually it seems I have misread the documentation. VOP3P still cannot have literals other than inline constants.
I will remove the part about the legalization, but need to keep isOperandLegal part.

Rebased.

In D37502#862146, @vpykhtin wrote:

LGTM. However it looks like we should fix TD to follow single naming convention.

Thanks!

LGTM

LGTM

LGTM