Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SystemZ] Dump function signature on missing arg extension. #109699

Merged
merged 1 commit into from
Sep 30, 2024

Conversation

JonPsson1
Copy link
Contributor

Make it easier to handle detected problems by providing the function
signature(s) involved in cases of missing argument extensions.

As suggested here: #109654

It would have been nice to have the actual (call/ret) instruction printed,
but it is not available, so this is the best I could do without imposing on
callers to LowerCall() / LowerReturn().

@llvmbot
Copy link

llvmbot commented Sep 23, 2024

@llvm/pr-subscribers-backend-systemz

Author: Jonas Paulsson (JonPsson1)

Changes

Make it easier to handle detected problems by providing the function
signature(s) involved in cases of missing argument extensions.

As suggested here: #109654

It would have been nice to have the actual (call/ret) instruction printed,
but it is not available, so this is the best I could do without imposing on
callers to LowerCall() / LowerReturn().


Full diff: https://github.com/llvm/llvm-project/pull/109699.diff

9 Files Affected:

  • (modified) llvm/lib/Target/SystemZ/SystemZISelLowering.cpp (+63-15)
  • (modified) llvm/lib/Target/SystemZ/SystemZISelLowering.h (+5-1)
  • (modified) llvm/test/CodeGen/SystemZ/args-15.ll (+3-1)
  • (modified) llvm/test/CodeGen/SystemZ/args-16.ll (+3-1)
  • (modified) llvm/test/CodeGen/SystemZ/args-17.ll (+3-1)
  • (modified) llvm/test/CodeGen/SystemZ/args-18.ll (+3-1)
  • (modified) llvm/test/CodeGen/SystemZ/args-19.ll (+3-1)
  • (modified) llvm/test/CodeGen/SystemZ/args-20.ll (+3-1)
  • (modified) llvm/test/CodeGen/SystemZ/args-21.ll (+3-1)
diff --git a/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp b/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
index 3dabc5ef540cfb..a7fdc3bc40fda0 100644
--- a/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
@@ -1925,11 +1925,7 @@ SystemZTargetLowering::LowerCall(CallLoweringInfo &CLI,
     IsTailCall = false;
 
   // Integer args <=32 bits should have an extension attribute.
-  bool IsInternal = false;
-  if (auto *G = dyn_cast<GlobalAddressSDNode>(Callee))
-    if (const Function *Fn = dyn_cast<Function>(G->getGlobal()))
-      IsInternal = isFullyInternal(Fn);
-  verifyNarrowIntegerArgs(Outs, IsInternal);
+  verifyNarrowIntegerArgs_Call(Outs, &MF.getFunction(), Callee);
 
   // Analyze the operands of the call, assigning locations to each operand.
   SmallVector<CCValAssign, 16> ArgLocs;
@@ -2192,7 +2188,7 @@ SystemZTargetLowering::LowerReturn(SDValue Chain, CallingConv::ID CallConv,
   MachineFunction &MF = DAG.getMachineFunction();
 
   // Integer args <=32 bits should have an extension attribute.
-  verifyNarrowIntegerArgs(Outs, isFullyInternal(&MF.getFunction()));
+  verifyNarrowIntegerArgs_Ret(Outs, &MF.getFunction());
 
   // Assign locations to each returned value.
   SmallVector<CCValAssign, 16> RetLocs;
@@ -9834,23 +9830,74 @@ bool SystemZTargetLowering::isFullyInternal(const Function *Fn) const {
   return true;
 }
 
-// Verify that narrow integer arguments are extended as required by the ABI.
+static void printFunctionArgExts(const Function *F, raw_fd_ostream &OS) {
+  FunctionType *FT = F->getFunctionType();
+  const AttributeList &Attrs = F->getAttributes();
+  if (Attrs.hasRetAttrs())
+    OS << Attrs.getAsString(AttributeList::ReturnIndex) << " ";
+  OS << *F->getReturnType() << " @" << F->getName() << "(";
+  for (unsigned I = 0, E = FT->getNumParams(); I != E; ++I) {
+    if (I)
+      OS << ", ";
+    OS << *FT->getParamType(I);
+    AttributeSet ArgAttrs = Attrs.getParamAttrs(I);
+    for (auto A : {Attribute::SExt, Attribute::ZExt, Attribute::NoExt})
+      if (ArgAttrs.hasAttribute(A))
+        OS << " " << Attribute::getNameFromAttrKind(A);
+  }
+  OS << ")\n";
+}
+
 void SystemZTargetLowering::
+verifyNarrowIntegerArgs_Call(const SmallVectorImpl<ISD::OutputArg> &Outs,
+                             const Function *F, SDValue Callee) const {
+  bool IsInternal = false;
+  const Function *CalleeFn = nullptr;
+  if (auto *G = dyn_cast<GlobalAddressSDNode>(Callee))
+    if (CalleeFn = dyn_cast<Function>(G->getGlobal()))
+      IsInternal = isFullyInternal(CalleeFn);
+  if (!verifyNarrowIntegerArgs(Outs, IsInternal)) {
+    errs() << "ERROR: Missing extension attribute of passed "
+           << "value in call to function:\n" << "Callee:  ";
+    if (CalleeFn != nullptr)
+      printFunctionArgExts(CalleeFn, errs());
+    else
+      errs() << "-";
+    errs() << "Caller:  ";
+    printFunctionArgExts(F, errs());
+    llvm_unreachable("");
+  }
+}
+
+void SystemZTargetLowering::
+verifyNarrowIntegerArgs_Ret(const SmallVectorImpl<ISD::OutputArg> &Outs,
+                            const Function *F) const {
+  if (!verifyNarrowIntegerArgs(Outs, isFullyInternal(F))) {
+    errs() << "ERROR: Missing extension attribute of returned "
+           << "value from function:\n";
+    printFunctionArgExts(F, errs());
+    llvm_unreachable("");
+  }
+}
+
+// Verify that narrow integer arguments are extended as required by the ABI.
+// Return false if an error is found.
+bool SystemZTargetLowering::
 verifyNarrowIntegerArgs(const SmallVectorImpl<ISD::OutputArg> &Outs,
                         bool IsInternal) const {
   if (IsInternal || !Subtarget.isTargetELF())
-    return;
+    return true;
 
   // Temporarily only do the check when explicitly requested, until it can be
   // enabled by default.
   if (!EnableIntArgExtCheck)
-    return;
+    return true;
 
   if (EnableIntArgExtCheck.getNumOccurrences()) {
     if (!EnableIntArgExtCheck)
-      return;
+      return true;
   } else if (!getTargetMachine().Options.VerifyArgABICompliance)
-    return;
+    return true;
 
   for (unsigned i = 0; i < Outs.size(); ++i) {
     MVT VT = Outs[i].VT;
@@ -9858,10 +9905,11 @@ verifyNarrowIntegerArgs(const SmallVectorImpl<ISD::OutputArg> &Outs,
     if (VT.isInteger()) {
       assert((VT == MVT::i32 || VT.getSizeInBits() >= 64) &&
              "Unexpected integer argument VT.");
-      assert((VT != MVT::i32 ||
-              (Flags.isSExt() || Flags.isZExt() || Flags.isNoExt())) &&
-             "Narrow integer argument must have a valid extension type.");
-      (void)Flags;
+      if (VT == MVT::i32 &&
+          !Flags.isSExt() && !Flags.isZExt() && !Flags.isNoExt())
+        return false;
     }
   }
+
+  return true;
 }
diff --git a/llvm/lib/Target/SystemZ/SystemZISelLowering.h b/llvm/lib/Target/SystemZ/SystemZISelLowering.h
index 8c528897182d17..2b065245c16f20 100644
--- a/llvm/lib/Target/SystemZ/SystemZISelLowering.h
+++ b/llvm/lib/Target/SystemZ/SystemZISelLowering.h
@@ -806,7 +806,11 @@ class SystemZTargetLowering : public TargetLowering {
   const TargetRegisterClass *getRepRegClassFor(MVT VT) const override;
 
   bool isFullyInternal(const Function *Fn) const;
-  void verifyNarrowIntegerArgs(const SmallVectorImpl<ISD::OutputArg> &Outs,
+  void verifyNarrowIntegerArgs_Call(const SmallVectorImpl<ISD::OutputArg> &Outs,
+                                    const Function *F, SDValue Callee) const;
+  void verifyNarrowIntegerArgs_Ret(const SmallVectorImpl<ISD::OutputArg> &Outs,
+                                   const Function *F) const;
+  bool verifyNarrowIntegerArgs(const SmallVectorImpl<ISD::OutputArg> &Outs,
                                bool IsInternal) const;
 };
 
diff --git a/llvm/test/CodeGen/SystemZ/args-15.ll b/llvm/test/CodeGen/SystemZ/args-15.ll
index c810aeb8c46c5d..64217a2a29a6f9 100644
--- a/llvm/test/CodeGen/SystemZ/args-15.ll
+++ b/llvm/test/CodeGen/SystemZ/args-15.ll
@@ -8,4 +8,6 @@ define i32 @callee_MissingRetAttr() {
   ret i32 -1
 }
 
-; CHECK: Narrow integer argument must have a valid extension type.
+; CHECK: ERROR: Missing extension attribute of returned value from function:
+; CHECK: i32 @callee_MissingRetAttr()
+; CHECK: UNREACHABLE executed
diff --git a/llvm/test/CodeGen/SystemZ/args-16.ll b/llvm/test/CodeGen/SystemZ/args-16.ll
index b76a2afea50775..846100146e7908 100644
--- a/llvm/test/CodeGen/SystemZ/args-16.ll
+++ b/llvm/test/CodeGen/SystemZ/args-16.ll
@@ -8,5 +8,7 @@ define i16 @callee_MissingRetAttr() {
   ret i16 -1
 }
 
-; CHECK: Narrow integer argument must have a valid extension type.
+; CHECK: ERROR: Missing extension attribute of returned value from function:
+; CHECK: i16 @callee_MissingRetAttr()
+; CHECK: UNREACHABLE executed
 
diff --git a/llvm/test/CodeGen/SystemZ/args-17.ll b/llvm/test/CodeGen/SystemZ/args-17.ll
index bce54b3d2aa1fe..4231d7e9e4772d 100644
--- a/llvm/test/CodeGen/SystemZ/args-17.ll
+++ b/llvm/test/CodeGen/SystemZ/args-17.ll
@@ -8,4 +8,6 @@ define i8 @callee_MissingRetAttr() {
   ret i8 -1
 }
 
-; CHECK: Narrow integer argument must have a valid extension type.
+; CHECK: ERROR: Missing extension attribute of returned value from function:
+; CHECK: i8 @callee_MissingRetAttr()
+; CHECK: UNREACHABLE executed
diff --git a/llvm/test/CodeGen/SystemZ/args-18.ll b/llvm/test/CodeGen/SystemZ/args-18.ll
index 82e9729d3a2dfd..bd368fa056c6c9 100644
--- a/llvm/test/CodeGen/SystemZ/args-18.ll
+++ b/llvm/test/CodeGen/SystemZ/args-18.ll
@@ -11,4 +11,6 @@ define void @caller() {
 
 declare void @bar_Struct(i32 %Arg)
 
-; CHECK: Narrow integer argument must have a valid extension type
+; CHECK: ERROR: Missing extension attribute of passed value in call to function:
+; CHECK: Callee:  void @bar_Struct(i32)
+; CHECK: Caller:  void @caller()
diff --git a/llvm/test/CodeGen/SystemZ/args-19.ll b/llvm/test/CodeGen/SystemZ/args-19.ll
index 40a794417b4c6f..8b5f421f59fdd8 100644
--- a/llvm/test/CodeGen/SystemZ/args-19.ll
+++ b/llvm/test/CodeGen/SystemZ/args-19.ll
@@ -11,4 +11,6 @@ define void @caller() {
 
 declare void @bar_Struct(i16 %Arg)
 
-; CHECK: Narrow integer argument must have a valid extension type
+; CHECK: ERROR: Missing extension attribute of passed value in call to function:
+; CHECK: Callee:  void @bar_Struct(i16)
+; CHECK: Caller:  void @caller()
diff --git a/llvm/test/CodeGen/SystemZ/args-20.ll b/llvm/test/CodeGen/SystemZ/args-20.ll
index ce8b828a2d539a..ed6f2e52bf6ee9 100644
--- a/llvm/test/CodeGen/SystemZ/args-20.ll
+++ b/llvm/test/CodeGen/SystemZ/args-20.ll
@@ -11,4 +11,6 @@ define void @caller() {
 
 declare void @bar_Struct(i8 %Arg)
 
-; CHECK: Narrow integer argument must have a valid extension type
+; CHECK: ERROR: Missing extension attribute of passed value in call to function:
+; CHECK: Callee:  void @bar_Struct(i8)
+; CHECK: Caller:  void @caller()
diff --git a/llvm/test/CodeGen/SystemZ/args-21.ll b/llvm/test/CodeGen/SystemZ/args-21.ll
index c64233094c7df9..da5c8fe5ffc7fc 100644
--- a/llvm/test/CodeGen/SystemZ/args-21.ll
+++ b/llvm/test/CodeGen/SystemZ/args-21.ll
@@ -16,4 +16,6 @@ define void @foo() {
   ret void
 }
 
-; CHECK: Narrow integer argument must have a valid extension type
+; CHECK: ERROR: Missing extension attribute of returned value from function:
+; CHECK: i32 @bar(i32)
+; CHECK: UNREACHABLE executed

Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 85220a0c651e565ab576c5be17c1ec5c9eb2a350 d89caff3f5095631c661698adaec43b95d00a0ee --extensions cpp,h -- llvm/lib/Target/SystemZ/SystemZISelLowering.cpp llvm/lib/Target/SystemZ/SystemZISelLowering.h
View the diff from clang-format here.
diff --git a/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp b/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
index a7fdc3bc40..ea53b7d632 100644
--- a/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
@@ -9848,9 +9848,9 @@ static void printFunctionArgExts(const Function *F, raw_fd_ostream &OS) {
   OS << ")\n";
 }
 
-void SystemZTargetLowering::
-verifyNarrowIntegerArgs_Call(const SmallVectorImpl<ISD::OutputArg> &Outs,
-                             const Function *F, SDValue Callee) const {
+void SystemZTargetLowering::verifyNarrowIntegerArgs_Call(
+    const SmallVectorImpl<ISD::OutputArg> &Outs, const Function *F,
+    SDValue Callee) const {
   bool IsInternal = false;
   const Function *CalleeFn = nullptr;
   if (auto *G = dyn_cast<GlobalAddressSDNode>(Callee))
@@ -9858,7 +9858,8 @@ verifyNarrowIntegerArgs_Call(const SmallVectorImpl<ISD::OutputArg> &Outs,
       IsInternal = isFullyInternal(CalleeFn);
   if (!verifyNarrowIntegerArgs(Outs, IsInternal)) {
     errs() << "ERROR: Missing extension attribute of passed "
-           << "value in call to function:\n" << "Callee:  ";
+           << "value in call to function:\n"
+           << "Callee:  ";
     if (CalleeFn != nullptr)
       printFunctionArgExts(CalleeFn, errs());
     else
@@ -9869,9 +9870,8 @@ verifyNarrowIntegerArgs_Call(const SmallVectorImpl<ISD::OutputArg> &Outs,
   }
 }
 
-void SystemZTargetLowering::
-verifyNarrowIntegerArgs_Ret(const SmallVectorImpl<ISD::OutputArg> &Outs,
-                            const Function *F) const {
+void SystemZTargetLowering::verifyNarrowIntegerArgs_Ret(
+    const SmallVectorImpl<ISD::OutputArg> &Outs, const Function *F) const {
   if (!verifyNarrowIntegerArgs(Outs, isFullyInternal(F))) {
     errs() << "ERROR: Missing extension attribute of returned "
            << "value from function:\n";
@@ -9882,9 +9882,8 @@ verifyNarrowIntegerArgs_Ret(const SmallVectorImpl<ISD::OutputArg> &Outs,
 
 // Verify that narrow integer arguments are extended as required by the ABI.
 // Return false if an error is found.
-bool SystemZTargetLowering::
-verifyNarrowIntegerArgs(const SmallVectorImpl<ISD::OutputArg> &Outs,
-                        bool IsInternal) const {
+bool SystemZTargetLowering::verifyNarrowIntegerArgs(
+    const SmallVectorImpl<ISD::OutputArg> &Outs, bool IsInternal) const {
   if (IsInternal || !Subtarget.isTargetELF())
     return true;
 
@@ -9905,8 +9904,8 @@ verifyNarrowIntegerArgs(const SmallVectorImpl<ISD::OutputArg> &Outs,
     if (VT.isInteger()) {
       assert((VT == MVT::i32 || VT.getSizeInBits() >= 64) &&
              "Unexpected integer argument VT.");
-      if (VT == MVT::i32 &&
-          !Flags.isSExt() && !Flags.isZExt() && !Flags.isNoExt())
+      if (VT == MVT::i32 && !Flags.isSExt() && !Flags.isZExt() &&
+          !Flags.isNoExt())
         return false;
     }
   }

Copy link
Member

@uweigand uweigand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@JonPsson1 JonPsson1 merged commit f9fbfc5 into llvm:main Sep 30, 2024
9 of 10 checks passed
@JonPsson1 JonPsson1 deleted the VerifyIntArgs branch September 30, 2024 15:03
@eugenis
Copy link
Contributor

eugenis commented Sep 30, 2024

This broke the -Werror build:
https://lab.llvm.org/buildbot/#/builders/51/builds/4527

@eugenis
Copy link
Contributor

eugenis commented Sep 30, 2024

Never mind, fixed already in df691ca

@JonPsson1
Copy link
Contributor Author

Never mind, fixed already in df691ca

thanks!

VitaNuo pushed a commit to VitaNuo/llvm-project that referenced this pull request Oct 2, 2024
)

Make it easier to handle detected problems by providing the function
signature(s) involved in cases of missing argument extensions.
VitaNuo pushed a commit to VitaNuo/llvm-project that referenced this pull request Oct 2, 2024
)

Make it easier to handle detected problems by providing the function
signature(s) involved in cases of missing argument extensions.
xgupta pushed a commit to xgupta/llvm-project that referenced this pull request Oct 4, 2024
)

Make it easier to handle detected problems by providing the function
signature(s) involved in cases of missing argument extensions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants