jlebar created this revision.
jlebar added reviewers: majnemer, rnk.
jlebar added subscribers: tra, jhen, cfe-commits.
We can't do the right thing, since there's no right thing to do, but at
least we can not crash the compiler.
http://reviews.llvm.org/D17103
Files:
tra added a comment.
Erasing an argument would only complicate the problem.
I guess for consistency we need to match clang's behavior for regular C++ code.
For optimized builds it just seems to pass NULL pointer instead.
http://reviews.llvm.org/D17103
hfinkel added a comment.
In http://reviews.llvm.org/D17103#349280, @jlebar wrote:
> > > I guess this is the part I'm unsure of. If it's legal to pass a struct
> > > to printf in regular C++ (seems to be?), I'd guess it should be legal in
> > > CUDA, too? I'm just not sure what it's supposed
hfinkel added a subscriber: hfinkel.
hfinkel added a comment.
In http://reviews.llvm.org/D17103#349182, @jlebar wrote:
> Yeah, I have no idea what's the right thing to do here. We can always pass a
> null pointer, that's easy. David, Reid, do you know what is the correct
> behavior?
I
hfinkel added a comment.
In http://reviews.llvm.org/D17103#349274, @jlebar wrote:
> > Ultimately, Sema should be responsible for rejecting this, correct?
>
>
> I guess this is the part I'm unsure of. If it's legal to pass a struct to
> printf in regular C++ (seems to be?), I'd guess it should
rnk added inline comments.
Comment at: lib/CodeGen/CGCUDABuiltin.cpp:105
@@ -99,3 +104,3 @@
llvm::Value *P = Builder.CreateStructGEP(AllocaTy, Alloca, I - 1);
llvm::Value *Arg = Args[I].RV.getScalarVal();
Builder.CreateAlignedStore(Arg, P,
jlebar added a comment.
Yeah, I have no idea what's the right thing to do here. We can always pass a
null pointer, that's easy. David, Reid, do you know what is the correct
behavior?
http://reviews.llvm.org/D17103
___
cfe-commits mailing list
hfinkel added a comment.
In http://reviews.llvm.org/D17103#349254, @jlebar wrote:
> In http://reviews.llvm.org/D17103#349245, @hfinkel wrote:
>
> > In http://reviews.llvm.org/D17103#349182, @jlebar wrote:
> >
> > > Yeah, I have no idea what's the right thing to do here. We can always
> > >
jlebar added a comment.
In http://reviews.llvm.org/D17103#349245, @hfinkel wrote:
> In http://reviews.llvm.org/D17103#349182, @jlebar wrote:
>
> > Yeah, I have no idea what's the right thing to do here. We can always pass
> > a null pointer, that's easy. David, Reid, do you know what is the
rnk added a comment.
Ultimately, Sema should be responsible for rejecting this, correct? In the
meantime we can have CodeGen reject this and emit a null value to avoid
crashing.
Comment at: lib/CodeGen/CGCUDABuiltin.cpp:105
@@ -99,3 +104,3 @@
llvm::Value *P =
jlebar updated this revision to Diff 47569.
jlebar added a comment.
Error out with CGM.ErrorUnsupported when we receive a non-scalar arg.
http://reviews.llvm.org/D17103
Files:
lib/CodeGen/CGCUDABuiltin.cpp
test/CodeGenCUDA/printf-aggregate.cu
Index: test/CodeGenCUDA/printf-aggregate.cu
This revision was automatically updated to reflect the committed changes.
jlebar marked an inline comment as done.
Closed by commit rL260479: [CUDA] Don't crash when trying to printf a
non-scalar object. (authored by jlebar).
Changed prior to commit:
jlebar added a comment.
OK, talked to Reid irl. Since this is just printf, not general varargs
handling, the Simplest Thing That Could Possibly Work is to error-unsupported.
Once we fix sema as described above, we can move the check there. Will update
the patch, thanks everyone.
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: lib/CodeGen/CGCUDABuiltin.cpp:90
@@ +89,3 @@
+CGM.ErrorUnsupported(E, "non-scalar arg to printf");
+return RValue::getIgnored();
+ }
14 matches
Mail list logo
