[PATCH] D122821: CallBase: fix getFnAttr so it also checks the function

[Augie Fackler via Phabricator via llvm-commits]

durin42 added inline comments.


================
Comment at: llvm/include/llvm/IR/InstrTypes.h:2308
 
+  template <typename AK> Attribute getFnAttrOnCalledFunction(AK Kind) const {
+    // Operand bundles override attributes on the called function, but don't
----------------
nikic wrote:
> durin42 wrote:
> > jyknight wrote:
> > > Probably best to put this fn next to hasFnAttrOnCalledFunction in the cc file?
> > But it's a template, so it has to be in the header, right?
> You can either make it a non-template (like the functions above), or you can add an explicit template instantiation in the source file, like so:
> 
> ```
> template Attribute getFnAttrOnCalledFunction(Attribute::AttrKind Kind) const;
> template Attribute getFnAttrOnCalledFunction(StringRef Kind) const;
> ```
I _think_ I've done what you wanted. If there's a way to get the implementation in the .cpp and the template instantiations in the .h I'm not aware of the trick and need a hand.


================
Comment at: llvm/test/Transforms/OpenMP/remove_globalization.ll:35
 ; CHECK-NEXT:    call void @bar() #[[ATTR0]]
-; CHECK-NEXT:    call void @unknown_no_openmp() #[[ATTR5:[0-9]+]]
+; CHECK-NEXT:    call void @unknown_no_openmp()
 ; CHECK-NEXT:    call void @__kmpc_target_deinit(%struct.ident_t* nonnull null, i8 1, i1 true)
----------------
nikic wrote:
> Hm, why do these test changes happen?
Because the attribute on the call was redundant (it's already on the definition). As far as I can tell the defect in getFnAttr() was hiding the attribute because of how llvm/lib/IR/Assumptions.cpp implements llvm::hasAssumption. I didn't spot the exact logic that adds this attribute to the call, but it seems clearly redundant to me in the IR on inspection.

Same goes for the other test file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122821