[PATCH] D42203: [AMDGPU] Scratch setup fix on AMDPAL gfx9+ merge shader

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 2 08:23:06 PST 2018


arsenm added inline comments.


================
Comment at: lib/Target/AMDGPU/SIFrameLowering.cpp:397
+          // ES+GS merge shader on gfx9+.
+          GitPtrLo = AMDGPU::SGPR8;
+          break;
----------------
Why aren't there corresponding changes in SIISelLowering? I would expect the argument lowering code would need to be aware of this


================
Comment at: test/CodeGen/AMDGPU/amdpal_scratch_mergeshader.ll:1
+; RUN: llc -mtriple=amdgcn--amdpal -mcpu=gfx900 -verify-machineinstrs < %s | FileCheck -check-prefix=CHECK -check-prefix=GFX9 -enable-var-scope %s
+
----------------
Use GCN instead of CHECK with multiple prefixes


================
Comment at: test/CodeGen/AMDGPU/amdpal_scratch_mergeshader.ll:12
+
+define dllexport amdgpu_hs void @_amdgpu_hs_main(i32 inreg %arg, i32 inreg %arg1, i32 inreg %arg2, i32 inreg %arg3, i32 inreg %arg4, i32 inreg %arg5, i32 inreg %arg6, i32 inreg %arg7, <6 x i32> inreg %arg8, i32 %arg9, i32 %arg10, i32 %arg11, i32 %arg12, i32 %arg13, i32 %arg14) #4 {
+.entry:
----------------
You don't need the linkage. Most of these arguments also look unused.

Why is most of this function body necessary? Shouldn't you only need a single volatile scratch access?


Repository:
  rL LLVM

https://reviews.llvm.org/D42203





More information about the llvm-commits mailing list