AMDGPU Benchmark Regression: Increased Assembler Length After Commit 54ec8bcaf85e

[michaelselehov]

Summary
After the commit 54ec8bc one of the AMDGPU benchmarks regressed because the assembler became longer.

Reduced Input IR
See input.ll in the attached archive

Steps to Reproduce

• Run opt with these parameters:
  opt -mtriple=amdgcn-amd-amdhsa -mcpu=gfx908 -O3 -vectorize-loops -vectorize-slp -amdgpu-early-inline-all=true -amdgpu-function-calls=false -S -o output.ll input.ll

• Then run llc with these parameters:
  llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx908 -O3 -vectorize-loops -vectorize-slp -amdgpu-early-inline-all=true -amdgpu-function-calls=false -o output.s output.ll

Additional Information
The input IR includes a complicated set of binary operations. The commit tries to fold the instructions but instead produces extra code, leading to the regression.

Please find the attachments for detailed comparison and further analysis.

input and output.zip

[michaelselehov]

@goldsteinn would you please take a look?

[llvmbot]

@llvm/issue-subscribers-backend-amdgpu

Author: None (michaelselehov)

**Summary** After the commit 54ec8bc one of the AMDGPU benchmarks regressed because the assembler became longer.

Reduced Input IR
See input.ll in the attached archive

Steps to Reproduce

• Run opt with these parameters:
  opt -mtriple=amdgcn-amd-amdhsa -mcpu=gfx908 -O3 -vectorize-loops -vectorize-slp -amdgpu-early-inline-all=true -amdgpu-function-calls=false -S -o output.ll input.ll

• Then run llc with these parameters:
  llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx908 -O3 -vectorize-loops -vectorize-slp -amdgpu-early-inline-all=true -amdgpu-function-calls=false -o output.s output.ll

Additional Information
The input IR includes a complicated set of binary operations. The commit tries to fold the instructions but instead produces extra code, leading to the regression.

Please find the attachments for detailed comparison and further analysis.

input and output.zip

[ronlieb]

@goldsteinn would you please take a look?

ping @goldsteinn

[jayfoad]

Just a drive-by comment: 54ec8bc seems pretty harmless to me and it would probably be more productive for an AMDGPU expert to look into why it exposed worse codegen.

[hidekisaito]

The issue appears to be related to the phase ordering of InstCombine against itself. Maybe, the issue can be investigated further with
opt -mtriple=amdgcn-amd-amdhsa -mcpu=gfx908 -passes=instcombine --debug -S input.ll

where input.ll is the following. As a result of the commit 54ec8bc, intermediate LLVM IR becomes the following form which InstCombine is unable to further optimize (most likely due to not having one the starting patterns to latch on).

; ModuleID = 'input.ll'
source_filename = "input.ll"
target datalayout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8"
target triple = "amdgcn-amd-amdhsa"

define half @foo(half %0, i16 %1) local_unnamed_addr #0 {
%3 = fptosi half %0 to i32
%4 = fmul half %0, 0xH0000
%5 = bitcast half %0 to i16
%6 = bitcast half %4 to i16
%7 = zext i16 %6 to i32
%8 = shl nuw i32 %7, 16
%9 = zext i16 %5 to i32
%10 = or i32 %8, %9
%11 = shl i32 %3, 4
%12 = and i32 %11, 16
%13 = lshr i32 %10, %12
%14 = shl i32 %3, 14
%15 = and i32 %14, 32768
%16 = and i16 %1, -32768
%17 = zext i16 %16 to i32
%18 = xor i32 %15, %17
%19 = xor i32 %18, %13
%20 = trunc i32 %19 to i16
%21 = bitcast i16 %20 to half
ret half %21
}

=================================
In the good case,
IC: Visiting: %6 = fmul half %0, 0xH0000
IC: Visiting: %7 = ...
IC: Visiting: %8 = bitcast half %6 to i16
IC: Visiting: %9 = zext i16 %8 to i32
IC: Visiting: %10 = shl nuw i32 %9, 16
eventually get
IC: Visiting: %8 = shl nuw i32 0, 16
IC: Replacing %8 = shl nuw i32 0, 16
with i32 0
IC: Mod = %8 = shl nuw i32 0, 16
New = %8 = shl nuw i32 0, 16
IC: ERASE %8 = shl nuw i32 0, 16
that enables further optimization

However, given the .ll input file above, InstCombine doesn't hit that part of the optimization.

[hidekisaito]

Good Case InstCombine:

; *** IR Dump After SimplifyCFGPass on foo ***
define half @foo(half %0, i16 %1) local_unnamed_addr #0 {
%3 = fptosi half %0 to i32
%4 = shl i32 %3, 16
%5 = and i32 %4, 196608
%6 = fmul half %0, 0xH0000
%7 = bitcast half %0 to i16
%8 = bitcast half %6 to i16
%9 = zext i16 %8 to i32
%10 = shl nuw i32 %9, 16
%11 = zext i16 %7 to i32
%12 = or disjoint i32 %10, %11
%13 = and i32 %5, 65536
%14 = icmp eq i32 %13, 0
%15 = select i1 %14, i32 %12, i32 %9
%16 = icmp samesign ugt i32 %5, 131071
%17 = select i1 %16, i32 32768, i32 0
%18 = and i16 %1, -32768
%19 = zext i16 %18 to i32
%20 = xor i32 %17, %19
%21 = xor i32 %20, %15
%22 = trunc i32 %21 to i16
%23 = bitcast i16 %22 to half
ret half %23
}
; *** IR Dump After InstCombinePass on foo ***
define half @foo(half %0, i16 %1) local_unnamed_addr #0 {
%3 = fptosi half %0 to i32
%4 = fmul half %0, 0xH0000
%5 = and i32 %3, 1
%6 = icmp eq i32 %5, 0
%.v.v = select i1 %6, half %0, half %4
%.v = bitcast half %.v.v to i16
%.tr = trunc i32 %3 to i16
%7 = shl i16 %.tr, 14
%8 = xor i16 %7, %1
%9 = and i16 %8, -32768
%10 = xor i16 %9, %.v
%11 = bitcast i16 %10 to half
ret half %11
}

[hidekisaito]

Bad Case InstCombine (input to this phase of InstCombine is different as a result of 54ec8bc)

; *** IR Dump After SimplifyCFGPass on foo ***
define half @foo(half %0, i16 %1) local_unnamed_addr #0 {
%3 = fptosi half %0 to i32
%4 = shl i32 %3, 16
%5 = and i32 %4, 196608
%6 = fmul half %0, 0xH0000
%7 = bitcast half %0 to i16
%8 = bitcast half %6 to i16
%9 = zext i16 %8 to i32
%10 = shl nuw i32 %9, 16
%11 = zext i16 %7 to i32
%12 = or disjoint i32 %10, %11
%13 = lshr i32 %5, 12
%14 = and i32 %13, 16
%15 = lshr i32 %12, %14
%16 = icmp samesign ugt i32 %5, 131071
%17 = select i1 %16, i32 32768, i32 0
%18 = and i16 %1, -32768
%19 = zext i16 %18 to i32
%20 = xor i32 %17, %19
%21 = xor i32 %20, %15
%22 = trunc i32 %21 to i16
%23 = bitcast i16 %22 to half
ret half %23
}
; *** IR Dump After InstCombinePass on foo ***
define half @foo(half %0, i16 %1) local_unnamed_addr #0 {
%3 = fptosi half %0 to i32
%4 = fmul half %0, 0xH0000
%5 = bitcast half %0 to i16
%6 = bitcast half %4 to i16
%7 = zext i16 %6 to i32
%8 = shl nuw i32 %7, 16
%9 = zext i16 %5 to i32
%10 = or disjoint i32 %8, %9
%11 = shl i32 %3, 4
%12 = and i32 %11, 16
%13 = lshr i32 %10, %12
%14 = shl i32 %3, 14
%15 = and i32 %14, 32768
%16 = and i16 %1, -32768
%17 = zext i16 %16 to i32
%18 = xor i32 %15, %17
%19 = xor i32 %18, %13
%20 = trunc i32 %19 to i16
%21 = bitcast i16 %20 to half
ret half %21
}

[zGoldthorpe]

Reran reproducer with latest upstream build, and the resulting assembly matches good_output.s.