[PATCH] D94585: [IndirectFunctions] Skip propagating attributes to address taken functions

[Madhur Amilkanthwar via Phabricator via llvm-commits]

madhur13490 added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPropagateAttributes.cpp:244
+          NewRoots.insert(&F);
+          Changed = true;
+        }
----------------
rampitec wrote:
> It is not really changed yet.
Ok


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUPropagateAttributes.cpp:272
+        // more than one function (called directly or indirectly).
+        if (Caller->hasAddressTaken() || CalleeProps == CallerProps) {
           if (!Roots.count(&F))
----------------
rampitec wrote:
> Do you still need this part? It was needed when you was doing partial update of the properties, which you do not do now.
Yes, it is still needed. Traversal may go into infinite loop, for example, the common-callees test depicts one scenario where we need this condition for convergence. 


================
Comment at: llvm/test/CodeGen/AMDGPU/propagate-attributes-direct-indirect.ll:77
+
+attributes #0 = { "amdgpu-flat-work-group-size"="1,256" "target-features"="+16-bit-insts,+add-no-carry-insts,+aperture-regs,+ci-insts,+dl-insts,+dot1-insts,+dot2-insts,+dpp,+ds-src2-insts,+enable-ds128,+enable-prt-strict-null,+fast-denormal-f32,+fast-fmaf,+flat-address-space,+flat-for-global,+flat-global-insts,+flat-inst-offsets,+flat-scratch-insts,+fma-mix-insts,+fp64,+gcn3-encoding,+gfx7-gfx8-gfx9-insts,+gfx8-insts,+gfx9,+gfx9-insts,+half-rate-64-ops,+image-gather4-d16-bug,+int-clamp-insts,+inv-2pi-inline-imm,+ldsbankcount32,+load-store-opt,+localmemorysize65536,+mad-mac-f32-insts,+no-xnack-support,+promote-alloca,+r128-a16,+s-memrealtime,+s-memtime-inst,+scalar-atomics,+scalar-flat-scratch-insts,+scalar-stores,+sdwa,+sdwa-omod,+sdwa-scalar,+sdwa-sdst,+sram-ecc,+trap-handler,+unaligned-access-mode,+unaligned-buffer-access,+unaligned-ds-access,+vgpr-index-mode,+vop3p,-wavefrontsize16,-wavefrontsize32,+wavefrontsize64,+xnack" }
+attributes #1 = { convergent norecurse nounwind mustprogress
----------------
rampitec wrote:
> This can be cleaned a lot.
How? These are features being propagated from direct callees.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94585