In D48929#4647755, @MaskRay wrote:

Note that the interaction with --compress-debug-sections={zlib,zstd} is not handled in this patch. For relocatable linking of .rel.debug_* sections, the implicit addends are not applied to the output.
I am trying to figure out a solution.

Writer<ELFT>::run
  maybeCompress
    OutputSection::writeTo
      InputSection::writeTo
        relocate  # relocations is empty
  writeSections
    OutputSection::writeTo
      InputSection::writeTo
        if SHT_REL copyRelocations
          addReloc R_ABS
        elif non-compress relocate
          relocateNonAllocForRelocatable handle R_ABS

Note that the interaction with --compress-debug-sections={zlib,zstd} is not handled in this patch. For relocatable linking of .rel.debug_* sections, the implicit addends are not applied to the output.
I am trying to figure out a solution.

LGTM if you want to either backport this to release/17.x or consider this risky and revert the prior patch just in release/17.x :)

LGTM.

Address Nemanja's comment by masking out bits 16-31 for both D-Form/DS-Forms.
Builds and tests successfully on both main and release/17.x.

Address comment to make hexadecimals lowercase.

Address review comments:

• Update checks for ppc64-tls-ie.s
• Mask out all of the XO bits for DS-Form

LGTM from my viewpoint if a PowerPC reviewer looks ok as well.

I've addressed all of the current comments, with the exception of Nemanja's comment on using a different mask for the DS-Form case (since we're still currently discussing that and I have asked for some clarification).

• Remove extra parentheses in PPC64.cpp
• Update test cases to have more unique names
• Make getPPCDSFormOp() return 0 for the default case

We will need to decide if we should backport this fix or if we should pull the original patch (which also has additional patches dependent on it that are in v17 as well).

Yes, rG598cccea80f5614869bf0dda4d09d68b2c64423c is also dependent on the original patch.

As Nemanja suggests, I will also apply this patch to the LLVM 17 release branch to test.

This patch fixes a regression caused by a patch that is in v17. We will need to decide if we should backport this fix or if we should pull the original patch (which also has additional patches dependent on it that are in v17 as well). I think this patch is obvious enough to be backported, but I'll defer the final decision on that to @MaskRay once this is approved.
@amyk Perhaps it would help to perform a thorough test with the approved version of this patch applied to v17 to ensure it is safe to backport.

Update getPPCDFormOp() to return the D-Form opcodes in the most significant bits (like getPPCDSFormOp().

Obsoleted by D119049

Thanks!

Final changes

LG, with the last few nits :)

Address comments

Address comments. Description update incoming...

In D123805#4506651, @ormris wrote:

ping

ping

In D155326#4502097, @dtcxzyw wrote:

In D155326#4501997, @jrtc27 wrote:

Why do you believe this is better than encoding it in the module's IR like the ABI?

• There is no module-level metadata for target CPU and sub-target features. So I just think that this patch is better than merging from func-level attributes like D142191.
• Clang passes -plugin-opt=mcpu to lld. Then lld builds lto::Config using codegen::getCPUStr and codegen::getMAttrs (both get the result from command args). It is taken for granted that clang passes -plugin-opt=mattr down to lld.

If we introduce top-level fields like target cpu and target features, we should modify a lot of things (clang/llvm/lld). It can take a long time to migrate (like opaque pointers).

-mattr is a different syntax than -march that users are used to. Why would we want to expose that? There was previously a proposal for doing this automatically https://reviews.llvm.org/D132843

In D155326#4501997, @jrtc27 wrote:

Why do you believe this is better than encoding it in the module's IR like the ABI?