[PATCH] D28538: [CodeGen] [CUDA] Add the ability set default attrs on functions in linked modules.

Justin Lebar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 13 22:31:30 PST 2017


jlebar planned changes to this revision.
jlebar added a comment.

Marking planned changes so I remember to see about that test, thanks for reminding me.



================
Comment at: clang/include/clang/Frontend/CodeGenOptions.h:136
+    std::string Filename;
+    /// If true, we set attributes functions in the bitcode library according to
+    /// our CodeGenOptions, much as we set attrs on functions that we generate
----------------
Note to self, typo.


================
Comment at: clang/include/clang/Frontend/CodeGenOptions.h:139
+    /// ourselves.
+    bool PropagateAttrs = false;
+    /// Bitwise combination of llvm::Linker::Flags, passed to the LLVM linker.
----------------
mehdi_amini wrote:
> Nit: I don't think "Propagate" is the right word, I feel something like "Override" would be better to describe what's happening.
Right now it doesn't override them, though.  It just adds the new attrs.  You're saying below that this is not safe, so maybe if we change that we should also change the name...


================
Comment at: clang/lib/CodeGen/CGCall.cpp:1720
+      getLLVMContext(), llvm::AttributeSet::FunctionIndex, FuncAttrs);
+  F.addAttributes(llvm::AttributeSet::FunctionIndex, AS);
+}
----------------
mehdi_amini wrote:
> Are we sure it always work? What if a function is annotated with an attribute incompatible with the attributes you're gonna add?
I have no idea -- what is going to happen?

In the CUDA use-case we make tons of assumptions about the contents of libdevice -- if we're making an additional assumption here, I'm fine with that.  Obviously we don't just want to nuke all of the attributes on the incoming function, because some of them might be derived from the IR as opposed to the codegen options.


https://reviews.llvm.org/D28538





More information about the llvm-commits mailing list