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

[AMDGPU] Add AMDGPU specific variadic operation MCExprs #82022

Merged
merged 8 commits into from
Mar 6, 2024

Conversation

JanekvO
Copy link
Contributor

@JanekvO JanekvO commented Feb 16, 2024

Adds AMDGPU specific variadic MCExpr operations 'max' and 'or'. Required for moving function/program resource usage information propagation to MC layer as propagation of the maximum and logic OR values will go through MCExprs. Propagation of said resource usage info through these MCExpr operations compared to current handling in AMDGPUResourceUsageAnalysis will ease the constraint on the order for a function's resource usage analysis as the symbolic definition of a resource can occur after its use.

@llvmbot llvmbot added backend:AMDGPU mc Machine (object) code labels Feb 16, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 16, 2024

@llvm/pr-subscribers-backend-amdgpu

@llvm/pr-subscribers-mc

Author: Janek van Oirschot (JanekvO)

Changes

Adds AMDGPU specific variadic MCExpr operations 'max' and 'or'. Required for moving function/program resource usage information propagation to MC layer as propagation of the maximum and logic OR values will go through MCExprs. Propagation of said resource usage info through these MCExpr operations compared to current handling in AMDGPUResourceUsageAnalysis will ease the constraint on the order for a function's resource usage analysis as the symbolic definition of a resource can occur after its use.


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

6 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp (+49)
  • (added) llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp (+92)
  • (added) llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h (+64)
  • (modified) llvm/lib/Target/AMDGPU/MCTargetDesc/CMakeLists.txt (+1)
  • (added) llvm/test/MC/AMDGPU/mcexpr_amd.s (+92)
  • (added) llvm/test/MC/AMDGPU/mcexpr_amd_err.s (+26)
diff --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index 79ad6ddf7861fc..c878d9d26988e9 100644
--- a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "AMDKernelCodeT.h"
+#include "MCTargetDesc/AMDGPUMCExpr.h"
 #include "MCTargetDesc/AMDGPUMCTargetDesc.h"
 #include "MCTargetDesc/AMDGPUTargetStreamer.h"
 #include "SIDefines.h"
@@ -1745,6 +1746,7 @@ class AMDGPUAsmParser : public MCTargetAsmParser {
 
 public:
   void onBeginOfFile() override;
+  bool parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) override;
 
   ParseStatus parseCustomOperand(OperandVector &Operands, unsigned MCK);
 
@@ -8130,6 +8132,53 @@ void AMDGPUAsmParser::onBeginOfFile() {
     getTargetStreamer().EmitDirectiveAMDGCNTarget();
 }
 
+/// Parse AMDGPU specific expressions.
+///
+///  expr ::= or(expr, ...) |
+///           max(expr, ...)
+///
+bool AMDGPUAsmParser::parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) {
+  using AGVK = AMDGPUVariadicMCExpr::AMDGPUVariadicKind;
+
+  auto parseVariadicExpr = [&](AGVK Kind, const MCExpr *&res, SMLoc &EndLoc) {
+    std::vector<const MCExpr *> Exprs;
+    while (true) {
+      if (isToken(AsmToken::RParen)) {
+        if (Exprs.empty()) {
+          Error(getToken().getLoc(), "empty max/or expression");
+          return true;
+        }
+        lex();
+        res = AMDGPUVariadicMCExpr::create(Kind, Exprs, getContext());
+        return false;
+      }
+      const MCExpr *Expr;
+      if (getParser().parseExpression(Expr, EndLoc))
+        return true;
+      Exprs.push_back(Expr);
+      if (!trySkipToken(AsmToken::Comma) && !isToken(AsmToken::RParen)) {
+        Error(getToken().getLoc(), "unexpected token in max/or expression");
+        return true;
+      }
+    }
+  };
+
+  if (isToken(AsmToken::Identifier)) {
+    StringRef TokenId = getTokenStr();
+    AGVK VK = StringSwitch<AGVK>(TokenId)
+                  .Case("max", AGVK::AGVK_Max)
+                  .Case("or", AGVK::AGVK_Or)
+                  .Default(AGVK::AGVK_None);
+
+    if (VK != AGVK::AGVK_None && peekToken().is(AsmToken::LParen)) {
+      lex(); // Eat 'max'/'or'
+      lex(); // Eat '('
+      return parseVariadicExpr(VK, Res, EndLoc);
+    }
+  }
+  return getParser().parsePrimaryExpr(Res, EndLoc, nullptr);
+}
+
 ParseStatus AMDGPUAsmParser::parseOModSI(OperandVector &Operands) {
   StringRef Name = getTokenStr();
   if (Name == "mul") {
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
new file mode 100644
index 00000000000000..3b00de3756d924
--- /dev/null
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
@@ -0,0 +1,92 @@
+//===- AMDGPUMCExpr.cpp - AMDGPU specific MC expression classes -----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "AMDGPUMCExpr.h"
+#include "llvm/MC/MCContext.h"
+#include "llvm/MC/MCStreamer.h"
+#include "llvm/MC/MCSymbol.h"
+#include "llvm/MC/MCValue.h"
+#include "llvm/Support/Allocator.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namespace llvm;
+
+const AMDGPUVariadicMCExpr *
+AMDGPUVariadicMCExpr::create(AMDGPUVariadicKind Kind,
+                             const std::vector<const MCExpr *> &Args,
+                             MCContext &Ctx) {
+  return new (Ctx) AMDGPUVariadicMCExpr(Kind, Args);
+}
+
+const MCExpr *AMDGPUVariadicMCExpr::getSubExpr(size_t index) const {
+  assert(index < Args.size() &&
+         "Indexing out of bounds AMDGPUVariadicMCExpr sub-expr");
+  return Args[index];
+}
+
+void AMDGPUVariadicMCExpr::printImpl(raw_ostream &OS,
+                                     const MCAsmInfo *MAI) const {
+  switch (Kind) {
+  default:
+    llvm_unreachable("Unknown AMDGPUVariadicMCExpr kind.");
+  case AGVK_Or:
+    OS << "or(";
+    break;
+  case AGVK_Max:
+    OS << "max(";
+    break;
+  }
+  for (auto it = Args.begin(); it != Args.end(); ++it) {
+    (*it)->print(OS, MAI, /*InParens=*/false);
+    if ((it + 1) != Args.end())
+      OS << ", ";
+  }
+  OS << ")";
+}
+
+bool AMDGPUVariadicMCExpr::evaluateAsRelocatableImpl(
+    MCValue &Res, const MCAsmLayout *Layout, const MCFixup *Fixup) const {
+  int64_t total = 0;
+
+  auto Op = [this](int64_t Arg1, int64_t Arg2) -> int64_t {
+    switch (Kind) {
+    default:
+      llvm_unreachable("Unknown AMDGPUVariadicMCExpr kind.");
+    case AGVK_Max:
+      return Arg1 > Arg2 ? Arg1 : Arg2;
+    case AGVK_Or:
+      return (!!Arg1) || (!!Arg2);
+    }
+  };
+
+  for (const MCExpr *Arg : Args) {
+    MCValue ArgRes;
+    if (!Arg->evaluateAsRelocatable(ArgRes, Layout, Fixup))
+      return false;
+    if (!ArgRes.isAbsolute())
+      return false;
+    total = Op(total, ArgRes.getConstant());
+  }
+
+  Res = MCValue::get(total);
+  return true;
+}
+
+void AMDGPUVariadicMCExpr::visitUsedExpr(MCStreamer &Streamer) const {
+  for (const MCExpr *Arg : Args) {
+    Streamer.visitUsedExpr(*Arg);
+  }
+}
+
+MCFragment *AMDGPUVariadicMCExpr::findAssociatedFragment() const {
+  for (const MCExpr *Arg : Args) {
+    if (Arg->findAssociatedFragment())
+      return Arg->findAssociatedFragment();
+  }
+  return nullptr;
+}
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
new file mode 100644
index 00000000000000..acd1dc47934bad
--- /dev/null
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
@@ -0,0 +1,64 @@
+//===- AMDGPUMCExpr.h - AMDGPU specific MC expression classes ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIB_TARGET_AMDGPU_MCTARGETDESC_AMDGPUMCEXPR_H
+#define LLVM_LIB_TARGET_AMDGPU_MCTARGETDESC_AMDGPUMCEXPR_H
+
+#include "llvm/MC/MCExpr.h"
+
+namespace llvm {
+
+class AMDGPUMCExpr : public MCTargetExpr {
+  static bool classof(const MCExpr *E) {
+    return E->getKind() == MCExpr::Target;
+  }
+  void fixELFSymbolsInTLSFixups(MCAssembler &) const override{};
+};
+
+class AMDGPUVariadicMCExpr : public AMDGPUMCExpr {
+public:
+  enum AMDGPUVariadicKind { AGVK_None, AGVK_Or, AGVK_Max };
+
+private:
+  AMDGPUVariadicKind Kind;
+  std::vector<const MCExpr *> Args;
+
+  AMDGPUVariadicMCExpr(AMDGPUVariadicKind Kind,
+                       const std::vector<const MCExpr *> &Args)
+      : Kind(Kind), Args(Args) {
+    assert(Args.size() >= 1 && "Can't take the maximum of 0 expressions.");
+  }
+
+public:
+  static const AMDGPUVariadicMCExpr *
+  create(AMDGPUVariadicKind Kind, const std::vector<const MCExpr *> &Args,
+         MCContext &Ctx);
+
+  static const AMDGPUVariadicMCExpr *
+  createOr(const std::vector<const MCExpr *> &Args, MCContext &Ctx) {
+    return create(AMDGPUVariadicKind::AGVK_Or, Args, Ctx);
+  }
+
+  static const AMDGPUVariadicMCExpr *
+  createMax(const std::vector<const MCExpr *> &Args, MCContext &Ctx) {
+    return create(AMDGPUVariadicKind::AGVK_Max, Args, Ctx);
+  }
+
+  AMDGPUVariadicKind getKind() const { return Kind; }
+  const MCExpr *getSubExpr(size_t index) const;
+
+  void printImpl(raw_ostream &OS, const MCAsmInfo *MAI) const override;
+  bool evaluateAsRelocatableImpl(MCValue &Res, const MCAsmLayout *Layout,
+                                 const MCFixup *Fixup) const override;
+  void visitUsedExpr(MCStreamer &Streamer) const override;
+  MCFragment *findAssociatedFragment() const override;
+};
+
+} // end namespace llvm
+
+#endif // LLVM_LIB_TARGET_AMDGPU_MCTARGETDESC_AMDGPUMCEXPR_H
\ No newline at end of file
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/CMakeLists.txt b/llvm/lib/Target/AMDGPU/MCTargetDesc/CMakeLists.txt
index 5dc76071b0594e..0842a58f794b32 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/CMakeLists.txt
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/CMakeLists.txt
@@ -5,6 +5,7 @@ add_llvm_component_library(LLVMAMDGPUDesc
   AMDGPUInstPrinter.cpp
   AMDGPUMCAsmInfo.cpp
   AMDGPUMCCodeEmitter.cpp
+  AMDGPUMCExpr.cpp
   AMDGPUMCTargetDesc.cpp
   AMDGPUTargetStreamer.cpp
   R600InstPrinter.cpp
diff --git a/llvm/test/MC/AMDGPU/mcexpr_amd.s b/llvm/test/MC/AMDGPU/mcexpr_amd.s
new file mode 100644
index 00000000000000..a201c35e8d2caf
--- /dev/null
+++ b/llvm/test/MC/AMDGPU/mcexpr_amd.s
@@ -0,0 +1,92 @@
+// RUN: llvm-mc -triple amdgcn-amd-amdhsa < %s | FileCheck --check-prefix=ASM %s
+// RUN: llvm-mc -triple amdgcn-amd-amdhsa -filetype=obj < %s > %t
+// RUN: llvm-objdump --syms %t | FileCheck --check-prefix=OBJDUMP %s
+
+// OBJDUMP: SYMBOL TABLE:
+// OBJDUMP-NEXT: 0000000000000000 l       *ABS*  0000000000000000 zero
+// OBJDUMP-NEXT: 0000000000000001 l       *ABS*  0000000000000000 one
+// OBJDUMP-NEXT: 0000000000000002 l       *ABS*  0000000000000000 two
+// OBJDUMP-NEXT: 0000000000000003 l       *ABS*  0000000000000000 three
+// OBJDUMP-NEXT: 0000000000000005 l       *ABS*  0000000000000000 max_expression_all
+// OBJDUMP-NEXT: 0000000000000005 l       *ABS*  0000000000000000 five
+// OBJDUMP-NEXT: 0000000000000004 l       *ABS*  0000000000000000 four
+// OBJDUMP-NEXT: 0000000000000002 l       *ABS*  0000000000000000 max_expression_two
+// OBJDUMP-NEXT: 0000000000000001 l       *ABS*  0000000000000000 max_expression_one
+// OBJDUMP-NEXT: 000000000000000a l       *ABS*  0000000000000000 max_literals
+// OBJDUMP-NEXT: 000000000000000f l       *ABS*  0000000000000000 max_with_max_sym
+// OBJDUMP-NEXT: 000000000000000f l       *ABS*  0000000000000000 max
+// OBJDUMP-NEXT: 0000000000000003 l       *ABS*  0000000000000000 max_with_subexpr
+// OBJDUMP-NEXT: 0000000000000006 l       *ABS*  0000000000000000 max_as_subexpr
+// OBJDUMP-NEXT: 0000000000000005 l       *ABS*  0000000000000000 max_recursive_subexpr
+// OBJDUMP-NEXT: 0000000000000001 l       *ABS*  0000000000000000 or_expression_all
+// OBJDUMP-NEXT: 0000000000000001 l       *ABS*  0000000000000000 or_expression_two
+// OBJDUMP-NEXT: 0000000000000001 l       *ABS*  0000000000000000 or_expression_one
+// OBJDUMP-NEXT: 0000000000000001 l       *ABS*  0000000000000000 or_literals
+// OBJDUMP-NEXT: 0000000000000000 l       *ABS*  0000000000000000 or_false
+// OBJDUMP-NEXT: 0000000000000001 l       *ABS*  0000000000000000 or_with_or_sym
+// OBJDUMP-NEXT: 00000000000000ff l       *ABS*  0000000000000000 or
+// OBJDUMP-NEXT: 0000000000000001 l       *ABS*  0000000000000000 or_with_subexpr
+// OBJDUMP-NEXT: 0000000000000002 l       *ABS*  0000000000000000 or_as_subexpr
+// OBJDUMP-NEXT: 0000000000000001 l       *ABS*  0000000000000000 or_recursive_subexpr
+
+// ASM: .set zero, 0
+// ASM: .set one, 1
+// ASM: .set two, 2
+// ASM: .set three, 3
+
+.set zero, 0
+.set one, 1
+.set two, 2
+.set three, 3
+
+// ASM: .set max_expression_all, max(1, 2, five, 3, four)
+// ASM: .set max_expression_two, 2
+// ASM: .set max_expression_one, 1
+// ASM: .set max_literals, 10
+// ASM: .set max_with_max_sym, max(max, 4, 3, 1, 2)
+
+.set max_expression_all, max(one, two, five, three, four)
+.set max_expression_two, max(one, two)
+.set max_expression_one, max(one)
+.set max_literals, max(1,2,3,4,5,6,7,8,9,10)
+.set max_with_max_sym, max(max, 4, 3, one, two)
+
+// ASM: .set max_with_subexpr, 3
+// ASM: .set max_as_subexpr, 1+(max(4, 3, five))
+// ASM: .set max_recursive_subexpr, max(max(1, four), 3, max_expression_all)
+
+.set max_with_subexpr, max(((one | 3) << 3) / 8)
+.set max_as_subexpr, 1 + max(4, 3, five)
+.set max_recursive_subexpr, max(max(one, four), three, max_expression_all)
+
+// ASM: .set or_expression_all, or(1, 2, five, 3, four)
+// ASM: .set or_expression_two, 1
+// ASM: .set or_expression_one, 1
+// ASM: .set or_literals, 1
+// ASM: .set or_false, 0
+// ASM: .set or_with_or_sym, or(or, 4, 3, 1, 2)
+
+.set or_expression_all, or(one, two, five, three, four)
+.set or_expression_two, or(one, two)
+.set or_expression_one, or(one)
+.set or_literals, or(1,2,3,4,5,6,7,8,9,10)
+.set or_false, or(zero, 0, (2-2), 5 > 6)
+.set or_with_or_sym, or(or, 4, 3, one, two)
+
+// ASM: .set or_with_subexpr, 1
+// ASM: .set or_as_subexpr, 1+(or(4, 3, five))
+// ASM: .set or_recursive_subexpr, or(or(1, four), 3, or_expression_all)
+
+.set or_with_subexpr, or(((one | 3) << 3) / 8)
+.set or_as_subexpr, 1 + or(4, 3, five)
+.set or_recursive_subexpr, or(or(one, four), three, or_expression_all)
+
+// ASM: .set four, 4
+// ASM: .set five, 5
+// ASM: .set max, 15
+// ASM: .set or, 255
+
+.set four, 4
+.set five, 5
+.set max, 0xF
+.set or, 0xFF
diff --git a/llvm/test/MC/AMDGPU/mcexpr_amd_err.s b/llvm/test/MC/AMDGPU/mcexpr_amd_err.s
new file mode 100644
index 00000000000000..ffe1918399a7cc
--- /dev/null
+++ b/llvm/test/MC/AMDGPU/mcexpr_amd_err.s
@@ -0,0 +1,26 @@
+// RUN: not llvm-mc -triple amdgcn-amd-amdhsa %s 2>&1 | FileCheck --check-prefix=ASM %s
+
+// ASM: error: empty max/or expression
+// ASM: error: missing expression
+// ASM: error: unknown token in expression
+// ASM: error: missing expression
+// ASM: error: unexpected token in max/or expression
+// ASM: error: missing expression
+// ASM: error: unknown token in expression
+// ASM: error: missing expression
+// ASM: error: unexpected token in max/or expression
+// ASM: error: missing expression
+
+.set one, 1
+.set two, 2
+.set three, 3
+
+.set max_empty, max()
+.set max_post_aux_comma, max(one,)
+.set max_pre_aux_comma, max(,one)
+.set max_missing_paren, max(two
+.set max_expression_one, max(three, four,
+.set max_expression_one, max(four, five
+
+.set four, 4
+.set five, 5

llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h Outdated Show resolved Hide resolved
llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h Outdated Show resolved Hide resolved
llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h Outdated Show resolved Hide resolved
llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h Outdated Show resolved Hide resolved
llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

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

Do we want these low level expressions, or do we want something that just combines the aggregate function resources?

llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp Outdated Show resolved Hide resolved
@JanekvO
Copy link
Contributor Author

JanekvO commented Feb 26, 2024

Do we want these low level expressions, or do we want something that just combines the aggregate function resources?

Yes, these are for combining the aggregate function resources. I'm also working on a patch dependent on this one for AMDGPUAsmPrinter::getSIProgramInfo to create aggregate MCExprs from AMDGPUResourceUsageAnalysis (i.e., places in said function where max operation is used on any of the (derived) results from AMDGPUResourceUsageAnalysis).

One of the reasons I'm using this approach is that constructing expressions is not dependent on the evaluability of sub-expressions (i.e., creating max(foo, bar) is fine and will be able to resolve for the object file through fixups regardless of order of occurrences for foo and bar).

Do let me know if there's an alternate approach I've missed.

llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp Outdated Show resolved Hide resolved
llvm/test/MC/AMDGPU/mcexpr_amd_err.s Outdated Show resolved Hide resolved
@JanekvO JanekvO requested a review from arsenm February 29, 2024 19:16
Copy link
Contributor

@Pierre-vh Pierre-vh left a comment

Choose a reason for hiding this comment

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

Already approving as this is good to me, just some minor style fixes needed.
Wait for @arsenm as well to approve.

Do you plan to merge this directly w/o any users, or merge it later alongside some code that uses those expressions?
I'd prefer to see some code that uses the expressions to be merged at around the same time, this avoids having dead code in the backend if the project gets dropped/changes direction/etc.

llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h Outdated Show resolved Hide resolved
llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h Outdated Show resolved Hide resolved
llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp Outdated Show resolved Hide resolved
@JanekvO
Copy link
Contributor Author

JanekvO commented Mar 1, 2024

Do you plan to merge this directly w/o any users, or merge it later alongside some code that uses those expressions? I'd prefer to see some code that uses the expressions to be merged at around the same time, this avoids having dead code in the backend if the project gets dropped/changes direction/etc.

I was hoping to merge so I can add appropriate tests in #80855 that demonstrate the propagation that could occur for kernel descriptors. I'll take responsibility of reverting in case this change won't be needed anymore.

Thanks for the review!

@JanekvO JanekvO requested a review from arsenm March 1, 2024 14:50
Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

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

Should this be documented in AMDGPUUsage?

llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp Outdated Show resolved Hide resolved
@JanekvO JanekvO requested a review from arsenm March 5, 2024 12:06
llvm/docs/AMDGPUUsage.rst Outdated Show resolved Hide resolved
llvm/docs/AMDGPUUsage.rst Show resolved Hide resolved
llvm/docs/AMDGPUUsage.rst Outdated Show resolved Hide resolved
@JanekvO JanekvO merged commit bec2d10 into llvm:main Mar 6, 2024
5 checks passed
@fmayer
Copy link
Contributor

fmayer commented Mar 7, 2024

The hwasan build bot discovered a leak in this: https://lab.llvm.org/buildbot/#/builders/236/builds/9874/steps/10/logs/stdio

FAIL: LLVM :: MC/AMDGPU/mcexpr_amd.s (51486 of 78334)
******************** TEST 'LLVM :: MC/AMDGPU/mcexpr_amd.s' FAILED ********************
Exit Code: 134
Command Output (stderr):
--
RUN: at line 1: /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm_build_hwasan/bin/llvm-mc -triple amdgcn-amd-amdhsa < /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/test/MC/AMDGPU/mcexpr_amd.s | /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm_build_hwasan/bin/FileCheck --check-prefix=ASM /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/test/MC/AMDGPU/mcexpr_amd.s
+ /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm_build_hwasan/bin/llvm-mc -triple amdgcn-amd-amdhsa
+ /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm_build_hwasan/bin/FileCheck --check-prefix=ASM /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/test/MC/AMDGPU/mcexpr_amd.s
=================================================================
==1084675==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 560 byte(s) in 12 object(s) allocated from:
    #0 0xaaaacd792484 in malloc /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/compiler-rt/lib/hwasan/hwasan_allocation_functions.cpp:151:3
    #1 0xaaaace59d814 in safe_malloc /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/include/llvm/Support/MemAlloc.h:26:18
    #2 0xaaaace59d814 in llvm::SmallVectorBase<unsigned int>::grow_pod(void*, unsigned long, unsigned long) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/Support/SmallVector.cpp:143:15
    #3 0xaaaacdcbd9b4 in grow_pod /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/include/llvm/ADT/SmallVector.h:151:11
    #4 0xaaaacdcbd9b4 in grow /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/include/llvm/ADT/SmallVector.h:538:41
    #5 0xaaaacdcbd9b4 in reserve /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/include/llvm/ADT/SmallVector.h:678:13
    #6 0xaaaacdcbd9b4 in void llvm::SmallVectorImpl<llvm::MCExpr const*>::append<llvm::MCExpr const* const*, void>(llvm::MCExpr const* const*, llvm::MCExpr const* const*) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/include/llvm/ADT/SmallVector.h:699:11
    #7 0xaaaacdcbcb50 in SmallVector<const llvm::MCExpr *, void> /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/include/llvm/ADT/SmallVector.h:1246:11
    #8 0xaaaacdcbcb50 in AMDGPUVariadicMCExpr /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h:37:21
    #9 0xaaaacdcbcb50 in llvm::AMDGPUVariadicMCExpr::create(llvm::AMDGPUVariadicMCExpr::VariadicKind, llvm::ArrayRef<llvm::MCExpr const*>, llvm::MCContext&) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp:23:20
    #10 0xaaaacd8a3980 in (anonymous namespace)::AMDGPUAsmParser::parsePrimaryExpr(llvm::MCExpr const*&, llvm::SMLoc&) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:8314:17
    #11 0xaaaace4b2b2c in (anonymous namespace)::AsmParser::parseExpression(llvm::MCExpr const*&, llvm::SMLoc&) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/MC/MCParser/AsmParser.cpp:1506:25
    #12 0xaaaace5213a0 in llvm::MCAsmParser::parseExpression(llvm::MCExpr const*&) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/MC/MCParser/MCAsmParser.cpp:142:10
    #13 0xaaaace4a4440 in llvm::MCParserUtils::parseAssignmentExpression(llvm::StringRef, bool, llvm::MCAsmParser&, llvm::MCSymbol*&, llvm::MCExpr const*&) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/MC/MCParser/AsmParser.cpp:6410:14
    #14 0xaaaace4c228c in (anonymous namespace)::AsmParser::parseAssignment(llvm::StringRef, (anonymous namespace)::AsmParser::AssignmentKind) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/MC/MCParser/AsmParser.cpp:2940:7
    #15 0xaaaace4c3b88 in (anonymous namespace)::AsmParser::parseDirectiveSet(llvm::StringRef, (anonymous namespace)::AsmParser::AssignmentKind) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/MC/MCParser/AsmParser.cpp:3026:7
    #16 0xaaaace4bcb38 in (anonymous namespace)::AsmParser::parseStatement((anonymous namespace)::ParseStatementInfo&, llvm::MCAsmParserSemaCallback*) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/MC/MCParser/AsmParser.cpp
    #17 0xaaaace4a91f8 in (anonymous namespace)::AsmParser::Run(bool, bool) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/MC/MCParser/AsmParser.cpp:1003:19
    #18 0xaaaacd7d4e90 in AssembleInput(char const*, llvm::Target const*, llvm::SourceMgr&, llvm::MCContext&, llvm::MCStreamer&, llvm::MCAsmInfo&, llvm::MCSubtargetInfo&, llvm::MCInstrInfo&, llvm::MCTargetOptions const&) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/tools/llvm-mc/llvm-mc.cpp:347:21
    #19 0xaaaacd7d1fe8 in main /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/tools/llvm-mc/llvm-mc.cpp:593:11
    #20 0xffffa7356dbc  (/lib/aarch64-linux-gnu/libc.so.6+0x26dbc) (BuildId: b3e2fd825ee86277a10a2c20b9fc836b101a2b7f)
    #21 0xffffa7356e94 in __libc_start_main (/lib/aarch64-linux-gnu/libc.so.6+0x26e94) (BuildId: b3e2fd825ee86277a10a2c20b9fc836b101a2b7f)
    #22 0xaaaacd78a6ac in _start (/b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm_build_hwasan/bin/llvm-mc+0x1eea6ac)
Direct leak of 80 byte(s) in 2 object(s) allocated from:
    #0 0xaaaacd792484 in malloc /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/compiler-rt/lib/hwasan/hwasan_allocation_functions.cpp:151:3
    #1 0xaaaace59d814 in safe_malloc /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/include/llvm/Support/MemAlloc.h:26:18
    #2 0xaaaace59d814 in llvm::SmallVectorBase<unsigned int>::grow_pod(void*, unsigned long, unsigned long) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/Support/SmallVector.cpp:143:15
    #3 0xaaaacdcbd9b4 in grow_pod /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/include/llvm/ADT/SmallVector.h:151:11
    #4 0xaaaacdcbd9b4 in grow /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/include/llvm/ADT/SmallVector.h:538:41
    #5 0xaaaacdcbd9b4 in reserve /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/include/llvm/ADT/SmallVector.h:678:13
    #6 0xaaaacdcbd9b4 in void llvm::SmallVectorImpl<llvm::MCExpr const*>::append<llvm::MCExpr const* const*, void>(llvm::MCExpr const* const*, llvm::MCExpr const* const*) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/include/llvm/ADT/SmallVector.h:699:11
    #7 0xaaaacdcbcb50 in SmallVector<const llvm::MCExpr *, void> /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/include/llvm/ADT/SmallVector.h:1246:11
    #8 0xaaaacdcbcb50 in AMDGPUVariadicMCExpr /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h:37:21
    #9 0xaaaacdcbcb50 in llvm::AMDGPUVariadicMCExpr::create(llvm::AMDGPUVariadicMCExpr::VariadicKind, llvm::ArrayRef<llvm::MCExpr const*>, llvm::MCContext&) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp:23:20
    #10 0xaaaacd8a3980 in (anonymous namespace)::AMDGPUAsmParser::parsePrimaryExpr(llvm::MCExpr const*&, llvm::SMLoc&) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp:8314:17
    #11 0xaaaace4f4184 in (anonymous namespace)::AsmParser::parseBinOpRHS(unsigned int, llvm::MCExpr const*&, llvm::SMLoc&) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/MC/MCParser/AsmParser.cpp:1765:27
    #12 0xaaaace4b2b94 in (anonymous namespace)::AsmParser::parseExpression(llvm::MCExpr const*&, llvm::SMLoc&) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/MC/MCParser/AsmParser.cpp:1507:7
    #13 0xaaaace5213a0 in llvm::MCAsmParser::parseExpression(llvm::MCExpr const*&) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/MC/MCParser/MCAsmParser.cpp:142:10
    #14 0xaaaace4a4440 in llvm::MCParserUtils::parseAssignmentExpression(llvm::StringRef, bool, llvm::MCAsmParser&, llvm::MCSymbol*&, llvm::MCExpr const*&) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/MC/MCParser/AsmParser.cpp:6410:14
    #15 0xaaaace4c228c in (anonymous namespace)::AsmParser::parseAssignment(llvm::StringRef, (anonymous namespace)::AsmParser::AssignmentKind) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/MC/MCParser/AsmParser.cpp:2940:7
    #16 0xaaaace4c3b88 in (anonymous namespace)::AsmParser::parseDirectiveSet(llvm::StringRef, (anonymous namespace)::AsmParser::AssignmentKind) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/MC/MCParser/AsmParser.cpp:3026:7
    #17 0xaaaace4bcb38 in (anonymous namespace)::AsmParser::parseStatement((anonymous namespace)::ParseStatementInfo&, llvm::MCAsmParserSemaCallback*) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/MC/MCParser/AsmParser.cpp
    #18 0xaaaace4a91f8 in (anonymous namespace)::AsmParser::Run(bool, bool) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/lib/MC/MCParser/AsmParser.cpp:1003:19
    #19 0xaaaacd7d4e90 in AssembleInput(char const*, llvm::Target const*, llvm::SourceMgr&, llvm::MCContext&, llvm::MCStreamer&, llvm::MCAsmInfo&, llvm::MCSubtargetInfo&, llvm::MCInstrInfo&, llvm::MCTargetOptions const&) /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/tools/llvm-mc/llvm-mc.cpp:347:21
    #20 0xaaaacd7d1fe8 in main /b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/tools/llvm-mc/llvm-mc.cpp:593:11
    #21 0xffffa7356dbc  (/lib/aarch64-linux-gnu/libc.so.6+0x26dbc) (BuildId: b3e2fd825ee86277a10a2c20b9fc836b101a2b7f)
    #22 0xffffa7356e94 in __libc_start_main (/lib/aarch64-linux-gnu/libc.so.6+0x26e94) (BuildId: b3e2fd825ee86277a10a2c20b9fc836b101a2b7f)
    #23 0xaaaacd78a6ac in _start (/b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm_build_hwasan/bin/llvm-mc+0x1eea6ac)
SUMMARY: HWAddressSanitizer: 640 byte(s) leaked in 14 allocation(s).
libc++abi: Pure virtual function called!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AMDGPU mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants