[PATCH] D9168: [NVPTX] Check if callsite is defined when computing argument allignment

Jacques Pienaar via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 20 08:35:48 PDT 2016


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, DL);
         // 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 clang-format-diff.py:
svn diff --diff-cmd=diff -x-U0 | ./tools/clang/tools/clang-format/clang-format-diff.py 

================
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)?


https://reviews.llvm.org/D9168





More information about the cfe-commits mailing list