LGTM

V3: AMDGCNGPUs table in TargetParser.cpp needs to be in GPUKind order,

so fix the GPUKind order.

V2: Add clang changes. Put TargetParser list in order.

+Chris Bieneman

I am abandoning this, because it turns out that it is difficult to reliably detect whether we have a PAL pipeline or a PAL shader compilation. Instead, the LLPC linker will remove the padding, and then adds it back on after concatenating .text sections.

Ping.

V2: Review comments: Separate predicate function; early exit on CS.

V5: API change so client can choose whether array merging does merge or append.

V4: Added MapDocNode::erase methods.

V2: Fixed tests.

V3: Added assignment for const char *, otherwise it defaults to using

the bool one rather than the StringRef one.

V2: Added tests

With the nil/empty change, there is a change in behavior in that doing a map lookup "map[key]" with a key not previously in the map creates a new map entry that is empty rather than nil. That is more correct, but I have just discovered some code in LLPC relying on the old behavior. I can fix the LLPC code, but is the change in behavior going to cause any HSA problems?

Ping

In D79671#2030708, @scott.linder wrote:

Dinesh is working on merging metadata in ROCm CompilerSupport using MsgPackDocument, and it fell out that merging YAML metadata was also handled transparently. What is the advantage to doing this via a function pointer passed to readFromBlob? It doesn't seem to save any copies as you still need to be able to compare the nodes; is it just a matter of readability? Would it be reasonable to also add a callback for fromYAML?

V3: Removed unneeded nullptr arg.

Ping.

V2: Pass map key to merger function.

V2: Also fixed GS regs. Added test.

I don't know of any reason why it has to be fp. Maybe related to the prehistoric thing where isel used to need something to be fp to put it in a vgpr?

This got landed in a slightly different form because of conflicts with other sdiv64 etc tests.

V2: Also added gfx1010 to memtime test.

Yes, sorry, I will do the test changes that Matt suggested so this can be approved and landed.

V2: Removed an existing test for the bogus fold.

V2: Sorted includes alphabetically.

In D70783#1762087, @rampitec wrote:

Did the problem manifest in any way?

No test because by definition it was indeterminate.

V3: Only bail if it is a copy and a partial def, to avoid spurious test changes.

V2: Used modifiesRegister as suggested by Matt.

This is slightly broken for us. See D70005 for a fix.

In D69557#1725564, @foad wrote:

In D69557#1725496, @spatel wrote:

Does D69161 cover this change? If so, we could add the test there and fix the problem in 1 step.

I think this bug fix is orthogonal to D69161, which adds new functionality.

V2: Better fix that does not accidentally allow pow() transforms.

@foad pointed out that this fix is wrong. TLI saying pow() is not supported means if we find a call to a function called pow() then we don't know its semantics. So I will push a revised fix.

In D67680#1673252, @arsenm wrote:

LGTM. Can you also add a test for the mad case