[PATCH] D38466: [ TargetLowering, AMDGPU] Use the return value of UpdateNodeOperands();

Mark Searles via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 2 12:46:03 PDT 2017


msearles marked 5 inline comments as done.
msearles added inline comments.


================
Comment at: test/CodeGen/AMDGPU/simplifydemandedbits-recursion.ll:1
+; RUN: (ulimit -t 5; llc -march=amdgcn < %s | FileCheck -check-prefix=GCN %s)
+
----------------
RKSimon wrote:
> arsenm wrote:
> > Does this actually work? I've never seen another test use ulimit or a sub shell. 5 seconds seems high also.
> Does this work on windows builds?
Nope; will not work on Windows; test adjusted.


================
Comment at: test/CodeGen/AMDGPU/simplifydemandedbits-recursion.ll:1
+; RUN: (ulimit -t 5; llc -march=amdgcn < %s | FileCheck -check-prefix=GCN %s)
+
----------------
msearles wrote:
> RKSimon wrote:
> > arsenm wrote:
> > > Does this actually work? I've never seen another test use ulimit or a sub shell. 5 seconds seems high also.
> > Does this work on windows builds?
> Nope; will not work on Windows; test adjusted.
Yes, it works quite nicely on Linux; it will not work on Windows (thanks Simon) as ulimit is not available. I removed the ulimit. So, potentially, there may be a compiler regression that causes this to loop infinitely; however, I think that you could say that about any test. 


================
Comment at: test/CodeGen/AMDGPU/simplifydemandedbits-recursion.ll:16
+
+; GCN: s_endpgm
+define amdgpu_kernel void @foo(float addrspace(1)* noalias nocapture readonly, float addrspace(1)* noalias nocapture readonly, float addrspace(1)* noalias nocapture, float) local_unnamed_addr !reqd_work_group_size !{i32 8, i32 16, i32 1} {
----------------
RKSimon wrote:
> You're not testing that foo() is empty - just that it includes s_endpgm
Updated the comments within the test; the purpose of the test is to verify that code was generated, not specifically that foo() is empty.



================
Comment at: test/CodeGen/AMDGPU/simplifydemandedbits-recursion.ll:74
+  %34 = phi i32 [ %11, %28 ], [ undef, %33 ]
+  %35 = getelementptr inbounds [462 x float], [462 x float] addrspace(3)* @0, i32 0, i32 %34
+  %36 = load float, float addrspace(3)* %35, align 4
----------------
arsenm wrote:
> Run instnamer on this test
Done.


https://reviews.llvm.org/D38466





More information about the llvm-commits mailing list