[PATCH] D157451: [AMDGPU] Do not deduce access qualifiers from IR attributes

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 9 11:44:09 PDT 2023


arsenm added inline comments.


================
Comment at: llvm/test/CodeGen/AMDGPU/hsa-metadata-deduce-ro-arg-v3.ll:2
 ; RUN: llc -mtriple=amdgcn-amd-amdhsa -filetype=obj -o - < %s | llvm-readelf --notes - | FileCheck %s
 
 ; CHECK:        - .args:
----------------
>From the test name, was this supposed to be inferred? Are there non-introspection uses for the access metadata?


================
Comment at: llvm/test/CodeGen/AMDGPU/hsa-metadata-deduce-ro-arg-v3.ll:26
 }
 
 !0 = !{i32 1, i32 1}
----------------
cfang wrote:
> arsenm wrote:
> > cfang wrote:
> > > arsenm wrote:
> > > > This looks like lost test coverage, need to test the access of the metadata
> > > Under V3, if the access is default, it won't print out "default", (will not print out the access qualifier at all).
> > > 
> > > But I am curious because "noalias readonly %in", why the access is not "readonly"? Does it mean the original access qualifier got lost? 
> > the point of this is introspection of the source program, not the actual properties. if someone defines something as readwrite but is only ever read, the optimizer can conclude it's readonly even though it wasn't declared that way
> Under V3 or above, it is designed NOT to print out the access qualifier if it is "default". 
> Do you want to CHECK-NOT for :access" for the default?
> 
> 
> 
-NEXT checks are safer than -NOT


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157451/new/

https://reviews.llvm.org/D157451



More information about the llvm-commits mailing list