This is an archive of the discontinued LLVM Phabricator instance.

WIP: AMDGPU: Use fixup for local linkage functions
Needs ReviewPublic

Authored by arsenm on Jun 5 2019, 8:29 AM.

Diff Detail

Event Timeline

arsenm created this revision.Jun 5 2019, 8:29 AM
dp added a subscriber: dp.Jul 22 2019, 11:36 AM

I contemplate if adding a 'proxy' instruction like s_add_u64 would help.
Code emitter would expand it into s_add+s_addc and handle relocations.
Assembly output would look like this:

s_add_u64 s[0:1], s[2:3], func
func:
  ...

encoding: [0x02,0xff,0x00,0x80,A,A,A,A,0x03,0xff,0x01,0x82,B,B,B,B]
          ;   fixup A - offset: 4, value: func, kind: FK_PCRel_4
          ;   fixup B - offset: 12, value: func, kind: fixup_si_pcrel_hi32
          ; <MCInst #10668 S_ADD_U64_gfx10
          ;  <MCOperand Reg:1832>
          ;  <MCOperand Reg:1833>
          ;  <MCOperand Expr:(func)>>

The code after relocation would be:

s_add_u32 s0, s2, 12              // 000000000028: 8000FF02 0000000C
s_addc_u32 s1, s3, 0              // 000000000030: 8201FF03 00000000
0000000000000038 func:
...

Advantages:

  • AMDGPUMCExpr.* are not needed;
  • no needsPCRel changes;
  • cleaner relocation handling;
  • cleaner codegen logic.

Disadvantages:

  • some hacks in SIMCCodeEmitter for proxy replacement.

Would this be useful?

I don’t want to use a pseudo instruction for this. We don’t need just add. We at least need sub, and there will be other possible users in the future

hliao added a subscriber: hliao.Jun 25 2020, 3:52 PM
hliao added inline comments.
lib/Target/AMDGPU/AMDGPUMCInstLower.cpp
156

Why not use MCBinaryExpr::createAShr to shift that high bits into low bits directly? We don't need invent a new target fixup.

arsenm marked an inline comment as done.Jul 6 2020, 2:10 PM
arsenm added inline comments.
lib/Target/AMDGPU/AMDGPUMCInstLower.cpp
156

That might work. Do we need an explicit truncate from 64-bit to 32-bit operator though?