jpienaar added a comment.

Cool. I didn't know the review system allows having the patch updated like this 
:) It still reports me as the author and you as a subscriber. I don't think 
that matters.

Comment at: lib/Target/NVPTX/NVPTXISelLowering.cpp:1033
@@ +1032,3 @@
+                                          const DataLayout &DL) const {
+  if (CS) {
+    unsigned Align = 0;
There is a preference to early exits. So perhaps:
  if (!CS)
    return DL.getABITTypeAlignment(Ty);

Comment at: lib/Target/NVPTX/NVPTXISelLowering.cpp:1131
@@ -1128,3 +1130,3 @@
-        unsigned align = getArgumentAlignment(Callee, CS, Ty, paramCount + 1);
+        unsigned align = getArgumentAlignment(Callee, CS, Ty, paramCount + 1, 
         // declare .param .align <align> .b8 .param<n>[<size>];
Move this to a new line to avoid exceeding 80 chars.

There are a couple of other formatting changes needed to. The simplest way is 
to use clang-format. To have the changes you added reformatted you could use
svn diff --diff-cmd=diff -x-U0 | 

Comment at: test/CodeGen/NVPTX/zero-cs.ll:1
@@ +1,2 @@
+; RUN: not llc < %s -march=nvptx 2>&1 | FileCheck %s
Could you add a comment explaining what this test is testing?

So this test case would fail previously (dereference null pointer) and now pass 
(return error)?

cfe-commits mailing list

Reply via email to