This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Fix counting source modifiers as literal constants
ClosedPublic

Authored by arsenm on Aug 4 2023, 8:04 AM.

Details

Reviewers
Joe_Nash
foad
rampitec
Pierre-vh
cdevadas
Group Reviewers
Restricted Project
Summary

This fixes over estimating code size. This was broken by
79f52af4cd9a76485dd50bcdbb5d393eb7a70103.

Diff Detail

Event Timeline

arsenm created this revision.Aug 4 2023, 8:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2023, 8:04 AM
arsenm requested review of this revision.Aug 4 2023, 8:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2023, 8:04 AM
Herald added a subscriber: wdng. · View Herald Transcript

On the other patch, you wrote "This change broke the handling of source modifiers. we're now treating any source modifier operand as a literal" You mean just the code size calculation is broken, right?

Can you please add test cases for GFX11, and something that emits fmamk/fmaak? Those were the motivating cases for the change.

On the other patch, you wrote "This change broke the handling of source modifiers. we're now treating any source modifier operand as a literal" You mean just the code size calculation is broken, right?

Can you please add test cases for GFX11, and something that emits fmamk/fmaak? Those were the motivating cases for the change.

Turns out those are broken. I think the API makes less sense now. "isInlineConstant" only makes sense for a vsrc operand and not an arbitary one. A counts-as-literal-use-something-in-any-operand would make more sense (inverted)

arsenm updated this revision to Diff 547313.Aug 4 2023, 12:36 PM

Explicitly handle all operand types and fix fmaak case

This revision is now accepted and ready to land.Aug 4 2023, 12:57 PM
foad added a comment.EditedAug 11 2023, 3:27 AM

4b1702e87a2687569b197aea4721353f8b788182

This is causing:

$ cat r.ll
define amdgpu_ps <2 x half> @f(half %arg) {
bb:
  %i = fmul contract half %arg, 0xH3A66
  %i1 = fadd contract half %i, 0xH2E66
  %i2 = fmul contract half %arg, 0xH0000
  %i3 = fsub contract half 0xH3A66, %i2
  %i4 = insertelement <2 x half> poison, half %i1, i32 0
  %i5 = insertelement <2 x half> %i4, half %i3, i32 1
  ret <2 x half> %i5
}
$ llc -march=amdgcn -mcpu=gfx1010 -o /dev/null r.ll 
error: Illegal instruction detected: VOP2/VOP3 instruction uses more than one literal
renamable $vgpr1 = contract nofpexcept V_FMAAK_F16 14950, $vgpr0, 11878, implicit $mode, implicit $exec

Can you please fix or revert?

4b1702e87a2687569b197aea4721353f8b788182

This is causing:

$ cat r.ll
define amdgpu_ps <2 x half> @f(half %arg) {
bb:
  %i = fmul contract half %arg, 0xH3A66
  %i1 = fadd contract half %i, 0xH2E66
  %i2 = fmul contract half %arg, 0xH0000
  %i3 = fsub contract half 0xH3A66, %i2
  %i4 = insertelement <2 x half> poison, half %i1, i32 0
  %i5 = insertelement <2 x half> %i4, half %i3, i32 1
  ret <2 x half> %i5
}
$ llc -march=amdgcn -mcpu=gfx1010 -o /dev/null r.ll 
error: Illegal instruction detected: VOP2/VOP3 instruction uses more than one literal
renamable $vgpr1 = contract nofpexcept V_FMAAK_F16 14950, $vgpr0, 11878, implicit $mode, implicit $exec

Can you please fix or revert?

I think https://reviews.llvm.org/D157624 will fix it.