[PATCH] D156397: [FunctionAttrs] Unconditionally perform argument attribute inference in the first function-attrs pass

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added inline comments.


================
Comment at: llvm/test/Transforms/PhaseOrdering/arg-attrs-affect-earlycse.ll:1
+;RUN: opt < %s -O3 -memssa-check-limit=1 -disable-output -debug-only=early-cse 2>&1 | FileCheck %s
+
----------------
nit: extra space before `RUN`


================
Comment at: llvm/test/Transforms/PhaseOrdering/arg-attrs-affect-earlycse.ll:3
+
+; CHECK: EarlyCSE CSE LOAD
+
----------------
I don't think it's a good idea in these phase ordering tests to specifically check that a specific pass did something, future pipeline changes can break this; the whole point of these is that the entire pipeline gives a good result regardless of exactly what's happening in the pipelines. This should use `llvm/utils/update_test_checks.py` like the other phase ordering tests to check the final IR. Although I tried that for this test and this patch doesn't change it, so I think you'll need to change this test to get something where this patch actually improves something. Testing with `-memssa-check-limit=1` is fine though.


================
Comment at: llvm/test/Transforms/PhaseOrdering/arg-attrs-affect-earlycse.ll:6
+; Function Attrs: convergent mustprogress norecurse nounwind
+define protected amdgpu_kernel void @arg_attrs_affect_earlycse(ptr noalias %arg0) #0 {
+bb0:
----------------
these function properties can be removed


================
Comment at: llvm/test/Transforms/PhaseOrdering/arg-attrs-affect-earlycse.ll:48
+
+define weak_odr hidden void @snork(i1 %arg) {
+bb0:
----------------
can be a declaration


================
Comment at: llvm/test/Transforms/PhaseOrdering/arg-attrs-affect-earlycse.ll:53
+
+attributes #0 = { convergent mustprogress norecurse nounwind  "target-features"="+wavefrontsize64"}
----------------
function attributes can be dropped


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

https://reviews.llvm.org/D156397