rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
Looks good with suggestions applied, and with the portability problems in the
test fixed. (Maybe just add a `-triple`? Though it would be good to also test
`inalloca` and ARM constructor `this` returns and so on.)
================
Comment at: clang/include/clang/Driver/Options.td:3957
+def disable_noundef_args : Flag<["-"], "disable-noundef-args">,
+ HelpText<"Disable emitting noundef attribute in LLVM IR">;
def load : Separate<["-"], "load">, MetaVarName<"<dsopath>">,
----------------
(since this doesn't affect return values)
================
Comment at: clang/lib/CodeGen/CGCall.cpp:2144
+ bool HasStrictReturn =
+ getLangOpts().CPlusPlus || getLangOpts().OpenCLCPlusPlus;
+ if (TargetDecl) {
----------------
`OpenCLCPlusPlus` should only ever be `true` when `CPlusPlus` is also `true`,
so the second half of this `||` should be unnecessary.
================
Comment at: clang/lib/CodeGen/CGCall.cpp:2148-2150
+ } else if (const VarDecl *VDecl = dyn_cast<VarDecl>(TargetDecl)) {
+ // Function pointer
+ HasStrictReturn &= !VDecl->isExternC();
----------------
`TargetDecl` (the callee of a function call) should never be a variable. You
shouldn't need this check.
================
Comment at: clang/test/CodeGen/attr-noundef.cpp:22
+// CHECK: define void @{{.*}}ret_nocopy{{.*}}(%"struct.check_structs::NoCopy"*
noalias sret align 4 %
+// CHECK: define void
@{{.*}}pass_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noundef %
+
----------------
I think this test will fail on 32-bit Windows because `e` will be passed
`inalloca`.
================
Comment at: clang/test/CodeGen/attr-noundef.cpp:78
+}
+// CHECK: define linkonce_odr void
@{{.*}}Object{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef i32
@{{.*}}Object{{.*}}getData{{.*}}(%"struct.check_this::Object"* noundef %
----------------
I believe this test will fail on ARM, due to constructors implicitly returning
`this`. Consider either specifying a target for this test or weakening the
`void` to `{{.*}}` here.
================
Comment at: clang/test/CodeGen/attr-noundef.cpp:79
+// CHECK: define linkonce_odr void
@{{.*}}Object{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef i32
@{{.*}}Object{{.*}}getData{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef %"struct.check_this::Object"*
@{{.*}}Object{{.*}}getThis{{.*}}(%"struct.check_this::Object"* noundef %
----------------
I believe this test will fail on Windows, because `getData` and `Object` will
appear in the opposite order in the mangling. Consider either specifying a
target or matching these names either way around.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81678/new/
https://reviews.llvm.org/D81678
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
