[llvm] Reland [AMDGPU] Add AMDGPU specific variadic operation MCExprs (PR #84562)

Janek van Oirschot via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 8 12:41:22 PST 2024


https://github.com/JanekvO updated https://github.com/llvm/llvm-project/pull/84562

>From 3d03a585c6dfb183f62cb65912917b21a96eeddc Mon Sep 17 00:00:00 2001
From: Janek van Oirschot <janek.vanoirschot at amd.com>
Date: Fri, 16 Feb 2024 17:01:44 +0000
Subject: [PATCH 01/10] AMDGPU Variadic MCExprs

---
 .../AMDGPU/AsmParser/AMDGPUAsmParser.cpp      | 49 ++++++++++
 .../AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp      | 92 +++++++++++++++++++
 .../Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h | 64 +++++++++++++
 .../Target/AMDGPU/MCTargetDesc/CMakeLists.txt |  1 +
 llvm/test/MC/AMDGPU/mcexpr_amd.s              | 92 +++++++++++++++++++
 llvm/test/MC/AMDGPU/mcexpr_amd_err.s          | 26 ++++++
 6 files changed, 324 insertions(+)
 create mode 100644 llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
 create mode 100644 llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
 create mode 100644 llvm/test/MC/AMDGPU/mcexpr_amd.s
 create mode 100644 llvm/test/MC/AMDGPU/mcexpr_amd_err.s

diff --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index cb4eddfe5320fa..a826d5cac3beac 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"
@@ -1816,6 +1817,7 @@ class AMDGPUAsmParser : public MCTargetAsmParser {
 
 public:
   void onBeginOfFile() override;
+  bool parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) override;
 
   ParseStatus parseCustomOperand(OperandVector &Operands, unsigned MCK);
 
@@ -8277,6 +8279,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

>From 7eaae11fe880e8cc1ae817607b7b9ad0e1a8861e Mon Sep 17 00:00:00 2001
From: Janek van Oirschot <janek.vanoirschot at amd.com>
Date: Mon, 19 Feb 2024 14:24:08 +0000
Subject: [PATCH 02/10] Apply feedback

---
 .../AMDGPU/AsmParser/AMDGPUAsmParser.cpp      | 12 ++---
 .../AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp      | 30 ++++++-------
 .../Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h | 45 ++++++++++---------
 llvm/test/MC/AMDGPU/mcexpr_amd.s              | 10 +++++
 4 files changed, 56 insertions(+), 41 deletions(-)

diff --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index a826d5cac3beac..48f970c93c4327 100644
--- a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -8287,16 +8287,16 @@ void AMDGPUAsmParser::onBeginOfFile() {
 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;
+  auto ParseVariadicExpr = [&](AGVK Kind, const MCExpr *&Result,
+                               SMLoc &EndLoc) {
+    SmallVector<const MCExpr *, 4> Exprs;
     while (true) {
-      if (isToken(AsmToken::RParen)) {
+      if (trySkipToken(AsmToken::RParen)) {
         if (Exprs.empty()) {
           Error(getToken().getLoc(), "empty max/or expression");
           return true;
         }
-        lex();
-        res = AMDGPUVariadicMCExpr::create(Kind, Exprs, getContext());
+        Result = AMDGPUVariadicMCExpr::create(Kind, Exprs, getContext());
         return false;
       }
       const MCExpr *Expr;
@@ -8320,7 +8320,7 @@ bool AMDGPUAsmParser::parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) {
     if (VK != AGVK::AGVK_None && peekToken().is(AsmToken::LParen)) {
       lex(); // Eat 'max'/'or'
       lex(); // Eat '('
-      return parseVariadicExpr(VK, Res, EndLoc);
+      return ParseVariadicExpr(VK, Res, EndLoc);
     }
   }
   return getParser().parsePrimaryExpr(Res, EndLoc, nullptr);
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
index 3b00de3756d924..013f2244449747 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
@@ -18,8 +18,7 @@ using namespace llvm;
 
 const AMDGPUVariadicMCExpr *
 AMDGPUVariadicMCExpr::create(AMDGPUVariadicKind Kind,
-                             const std::vector<const MCExpr *> &Args,
-                             MCContext &Ctx) {
+                             ArrayRef<const MCExpr *> Args, MCContext &Ctx) {
   return new (Ctx) AMDGPUVariadicMCExpr(Kind, Args);
 }
 
@@ -41,9 +40,9 @@ void AMDGPUVariadicMCExpr::printImpl(raw_ostream &OS,
     OS << "max(";
     break;
   }
-  for (auto it = Args.begin(); it != Args.end(); ++it) {
-    (*it)->print(OS, MAI, /*InParens=*/false);
-    if ((it + 1) != Args.end())
+  for (auto It = Args.begin(); It != Args.end(); ++It) {
+    (*It)->print(OS, MAI, /*InParens=*/false);
+    if ((It + 1) != Args.end())
       OS << ", ";
   }
   OS << ")";
@@ -51,36 +50,37 @@ void AMDGPUVariadicMCExpr::printImpl(raw_ostream &OS,
 
 bool AMDGPUVariadicMCExpr::evaluateAsRelocatableImpl(
     MCValue &Res, const MCAsmLayout *Layout, const MCFixup *Fixup) const {
-  int64_t total = 0;
+  int64_t Total = INT64_MIN;
 
   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;
+      return std::max(Arg1, Arg2);
     case AGVK_Or:
-      return (!!Arg1) || (!!Arg2);
+      return Arg1 || Arg2;
     }
   };
 
   for (const MCExpr *Arg : Args) {
     MCValue ArgRes;
-    if (!Arg->evaluateAsRelocatable(ArgRes, Layout, Fixup))
+    if (!Arg->evaluateAsRelocatable(ArgRes, Layout, Fixup) ||
+        !ArgRes.isAbsolute())
       return false;
-    if (!ArgRes.isAbsolute())
-      return false;
-    total = Op(total, ArgRes.getConstant());
+
+    if (Total == INT64_MIN)
+      Total = ArgRes.getConstant();
+    Total = Op(Total, ArgRes.getConstant());
   }
 
-  Res = MCValue::get(total);
+  Res = MCValue::get(Total);
   return true;
 }
 
 void AMDGPUVariadicMCExpr::visitUsedExpr(MCStreamer &Streamer) const {
-  for (const MCExpr *Arg : Args) {
+  for (const MCExpr *Arg : Args)
     Streamer.visitUsedExpr(*Arg);
-  }
 }
 
 MCFragment *AMDGPUVariadicMCExpr::findAssociatedFragment() const {
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
index acd1dc47934bad..a1657adf8ca7c6 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
@@ -9,43 +9,44 @@
 #ifndef LLVM_LIB_TARGET_AMDGPU_MCTARGETDESC_AMDGPUMCEXPR_H
 #define LLVM_LIB_TARGET_AMDGPU_MCTARGETDESC_AMDGPUMCEXPR_H
 
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallVector.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 {
+/// AMDGPU target specific variadic MCExpr operations.
+///
+/// Takes in a minimum of 1 argument to be used with an operation. The supported
+/// operations are:
+///   - (logic) or
+///   - max
+///
+class AMDGPUVariadicMCExpr : public MCTargetExpr {
 public:
   enum AMDGPUVariadicKind { AGVK_None, AGVK_Or, AGVK_Max };
 
 private:
   AMDGPUVariadicKind Kind;
-  std::vector<const MCExpr *> Args;
+  SmallVector<const MCExpr *, 2> Args;
 
-  AMDGPUVariadicMCExpr(AMDGPUVariadicKind Kind,
-                       const std::vector<const MCExpr *> &Args)
+  AMDGPUVariadicMCExpr(AMDGPUVariadicKind Kind, ArrayRef<const MCExpr *> Args)
       : Kind(Kind), Args(Args) {
-    assert(Args.size() >= 1 && "Can't take the maximum of 0 expressions.");
+    assert(Args.size() >= 1 && "Needs a minimum of one expression.");
   }
 
 public:
-  static const AMDGPUVariadicMCExpr *
-  create(AMDGPUVariadicKind Kind, const std::vector<const MCExpr *> &Args,
-         MCContext &Ctx);
+  static const AMDGPUVariadicMCExpr *create(AMDGPUVariadicKind Kind,
+                                            ArrayRef<const MCExpr *> Args,
+                                            MCContext &Ctx);
 
-  static const AMDGPUVariadicMCExpr *
-  createOr(const std::vector<const MCExpr *> &Args, MCContext &Ctx) {
+  static const AMDGPUVariadicMCExpr *createOr(ArrayRef<const MCExpr *> Args,
+                                              MCContext &Ctx) {
     return create(AMDGPUVariadicKind::AGVK_Or, Args, Ctx);
   }
 
-  static const AMDGPUVariadicMCExpr *
-  createMax(const std::vector<const MCExpr *> &Args, MCContext &Ctx) {
+  static const AMDGPUVariadicMCExpr *createMax(ArrayRef<const MCExpr *> Args,
+                                               MCContext &Ctx) {
     return create(AMDGPUVariadicKind::AGVK_Max, Args, Ctx);
   }
 
@@ -57,8 +58,12 @@ class AMDGPUVariadicMCExpr : public AMDGPUMCExpr {
                                  const MCFixup *Fixup) const override;
   void visitUsedExpr(MCStreamer &Streamer) const override;
   MCFragment *findAssociatedFragment() const override;
+  static bool classof(const MCExpr *E) {
+    return E->getKind() == MCExpr::Target;
+  }
+  void fixELFSymbolsInTLSFixups(MCAssembler &) const override{};
 };
 
 } // end namespace llvm
 
-#endif // LLVM_LIB_TARGET_AMDGPU_MCTARGETDESC_AMDGPUMCEXPR_H
\ No newline at end of file
+#endif // LLVM_LIB_TARGET_AMDGPU_MCTARGETDESC_AMDGPUMCEXPR_H
diff --git a/llvm/test/MC/AMDGPU/mcexpr_amd.s b/llvm/test/MC/AMDGPU/mcexpr_amd.s
index a201c35e8d2caf..0ebcbc08363a8a 100644
--- a/llvm/test/MC/AMDGPU/mcexpr_amd.s
+++ b/llvm/test/MC/AMDGPU/mcexpr_amd.s
@@ -15,6 +15,9 @@
 // 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: ffffffffffffffff l       *ABS*  0000000000000000 neg_one
+// OBJDUMP-NEXT: ffffffffffffffff l       *ABS*  0000000000000000 max_neg_numbers
+// OBJDUMP-NEXT: ffffffffffffffff l       *ABS*  0000000000000000 max_neg_number
 // 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
@@ -51,6 +54,13 @@
 .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_neg_numbers, -1
+// ASM: .set max_neg_number, -1
+
+.set neg_one, -1
+.set max_neg_numbers, max(-5, -4, -3, -2, neg_one)
+.set max_neg_number, max(neg_one)
+
 // 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)

>From d9b0a9b39c6144d8a935293b1d7153959168ec93 Mon Sep 17 00:00:00 2001
From: Janek van Oirschot <janek.vanoirschot at amd.com>
Date: Mon, 26 Feb 2024 14:53:22 +0000
Subject: [PATCH 03/10] Feedback, use optional for MCExpr instead of INT64_MIN

---
 .../AMDGPU/AsmParser/AMDGPUAsmParser.cpp      | 45 +++++++++----------
 .../AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp      | 11 ++---
 .../Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h |  8 +++-
 llvm/test/MC/AMDGPU/mcexpr_amd.s              | 28 ++++++++++++
 llvm/test/MC/AMDGPU/mcexpr_amd_err.s          | 11 +++--
 5 files changed, 68 insertions(+), 35 deletions(-)

diff --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index 48f970c93c4327..18527e103a66af 100644
--- a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -8287,29 +8287,6 @@ void AMDGPUAsmParser::onBeginOfFile() {
 bool AMDGPUAsmParser::parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) {
   using AGVK = AMDGPUVariadicMCExpr::AMDGPUVariadicKind;
 
-  auto ParseVariadicExpr = [&](AGVK Kind, const MCExpr *&Result,
-                               SMLoc &EndLoc) {
-    SmallVector<const MCExpr *, 4> Exprs;
-    while (true) {
-      if (trySkipToken(AsmToken::RParen)) {
-        if (Exprs.empty()) {
-          Error(getToken().getLoc(), "empty max/or expression");
-          return true;
-        }
-        Result = 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)
@@ -8318,9 +8295,29 @@ bool AMDGPUAsmParser::parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) {
                   .Default(AGVK::AGVK_None);
 
     if (VK != AGVK::AGVK_None && peekToken().is(AsmToken::LParen)) {
+      SmallVector<const MCExpr *, 4> Exprs;
       lex(); // Eat 'max'/'or'
       lex(); // Eat '('
-      return ParseVariadicExpr(VK, Res, EndLoc);
+      while (true) {
+        if (trySkipToken(AsmToken::RParen)) {
+          if (Exprs.empty()) {
+            Error(getToken().getLoc(),
+                  "empty " + Twine(TokenId) + " expression");
+            return true;
+          }
+          Res = AMDGPUVariadicMCExpr::create(VK, 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 " + Twine(TokenId) + " expression");
+          return true;
+        }
+      }
     }
   }
   return getParser().parsePrimaryExpr(Res, EndLoc, nullptr);
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
index 013f2244449747..5bd188aae975f1 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
@@ -13,6 +13,7 @@
 #include "llvm/MC/MCValue.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/raw_ostream.h"
+#include <optional>
 
 using namespace llvm;
 
@@ -22,7 +23,7 @@ AMDGPUVariadicMCExpr::create(AMDGPUVariadicKind Kind,
   return new (Ctx) AMDGPUVariadicMCExpr(Kind, Args);
 }
 
-const MCExpr *AMDGPUVariadicMCExpr::getSubExpr(size_t index) const {
+const MCExpr *AMDGPUVariadicMCExpr::GetSubExpr(size_t index) const {
   assert(index < Args.size() &&
          "Indexing out of bounds AMDGPUVariadicMCExpr sub-expr");
   return Args[index];
@@ -50,7 +51,7 @@ void AMDGPUVariadicMCExpr::printImpl(raw_ostream &OS,
 
 bool AMDGPUVariadicMCExpr::evaluateAsRelocatableImpl(
     MCValue &Res, const MCAsmLayout *Layout, const MCFixup *Fixup) const {
-  int64_t Total = INT64_MIN;
+  std::optional<int64_t> Total = {};
 
   auto Op = [this](int64_t Arg1, int64_t Arg2) -> int64_t {
     switch (Kind) {
@@ -69,12 +70,12 @@ bool AMDGPUVariadicMCExpr::evaluateAsRelocatableImpl(
         !ArgRes.isAbsolute())
       return false;
 
-    if (Total == INT64_MIN)
+    if (!Total.has_value())
       Total = ArgRes.getConstant();
-    Total = Op(Total, ArgRes.getConstant());
+    Total = Op(*Total, ArgRes.getConstant());
   }
 
-  Res = MCValue::get(Total);
+  Res = MCValue::get(*Total);
   return true;
 }
 
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
index a1657adf8ca7c6..1e39810b0ea924 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
@@ -22,6 +22,9 @@ namespace llvm {
 ///   - (logic) or
 ///   - max
 ///
+/// \note If the 'or'/'max' operations are provided only a single argument, the
+/// operation will act as a no-op and simply resolve as the provided argument.
+///
 class AMDGPUVariadicMCExpr : public MCTargetExpr {
 public:
   enum AMDGPUVariadicKind { AGVK_None, AGVK_Or, AGVK_Max };
@@ -51,17 +54,18 @@ class AMDGPUVariadicMCExpr : public MCTargetExpr {
   }
 
   AMDGPUVariadicKind getKind() const { return Kind; }
-  const MCExpr *getSubExpr(size_t index) const;
+  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;
+  void fixELFSymbolsInTLSFixups(MCAssembler &) const override{};
+
   static bool classof(const MCExpr *E) {
     return E->getKind() == MCExpr::Target;
   }
-  void fixELFSymbolsInTLSFixups(MCAssembler &) const override{};
 };
 
 } // end namespace llvm
diff --git a/llvm/test/MC/AMDGPU/mcexpr_amd.s b/llvm/test/MC/AMDGPU/mcexpr_amd.s
index 0ebcbc08363a8a..3e3d6057318e54 100644
--- a/llvm/test/MC/AMDGPU/mcexpr_amd.s
+++ b/llvm/test/MC/AMDGPU/mcexpr_amd.s
@@ -7,6 +7,8 @@
 // OBJDUMP-NEXT: 0000000000000001 l       *ABS*  0000000000000000 one
 // OBJDUMP-NEXT: 0000000000000002 l       *ABS*  0000000000000000 two
 // OBJDUMP-NEXT: 0000000000000003 l       *ABS*  0000000000000000 three
+// OBJDUMP-NEXT: 7fffffffffffffff l       *ABS*  0000000000000000 i64_max
+// OBJDUMP-NEXT: 8000000000000000 l       *ABS*  0000000000000000 i64_min
 // OBJDUMP-NEXT: 0000000000000005 l       *ABS*  0000000000000000 max_expression_all
 // OBJDUMP-NEXT: 0000000000000005 l       *ABS*  0000000000000000 five
 // OBJDUMP-NEXT: 0000000000000004 l       *ABS*  0000000000000000 four
@@ -21,6 +23,12 @@
 // 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: 7fffffffffffffff l       *ABS*  0000000000000000 max_expr_one_max
+// OBJDUMP-NEXT: 7fffffffffffffff l       *ABS*  0000000000000000 max_expr_two_max
+// OBJDUMP-NEXT: 7fffffffffffffff l       *ABS*  0000000000000000 max_expr_three_max
+// OBJDUMP-NEXT: 8000000000000000 l       *ABS*  0000000000000000 max_expr_one_min
+// OBJDUMP-NEXT: 0000000000000003 l       *ABS*  0000000000000000 max_expr_two_min
+// OBJDUMP-NEXT: 0000000000989680 l       *ABS*  0000000000000000 max_expr_three_min
 // 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
@@ -36,11 +44,15 @@
 // ASM: .set one, 1
 // ASM: .set two, 2
 // ASM: .set three, 3
+// ASM: .set i64_max, 9223372036854775807
+// ASM: .set i64_min, -9223372036854775808
 
 .set zero, 0
 .set one, 1
 .set two, 2
 .set three, 3
+.set i64_max, 0x7FFFFFFFFFFFFFFF
+.set i64_min, 0x8000000000000000
 
 // ASM: .set max_expression_all, max(1, 2, five, 3, four)
 // ASM: .set max_expression_two, 2
@@ -69,6 +81,22 @@
 .set max_as_subexpr, 1 + max(4, 3, five)
 .set max_recursive_subexpr, max(max(one, four), three, max_expression_all)
 
+// ASM: .set max_expr_one_max, 9223372036854775807
+// ASM: .set max_expr_two_max, max(9223372036854775807, five)
+// ASM: .set max_expr_three_max, max(9223372036854775807, five, 10000000)
+
+.set max_expr_one_max, max(i64_max)
+.set max_expr_two_max, max(i64_max, five)
+.set max_expr_three_max, max(i64_max, five, 10000000)
+
+// ASM: .set max_expr_one_min, -9223372036854775808
+// ASM: .set max_expr_two_min, 3
+// ASM: .set max_expr_three_min, 10000000
+
+.set max_expr_one_min, max(i64_min)
+.set max_expr_two_min, max(i64_min, three)
+.set max_expr_three_min, max(i64_min, three, 10000000)
+
 // ASM: .set or_expression_all, or(1, 2, five, 3, four)
 // ASM: .set or_expression_two, 1
 // ASM: .set or_expression_one, 1
diff --git a/llvm/test/MC/AMDGPU/mcexpr_amd_err.s b/llvm/test/MC/AMDGPU/mcexpr_amd_err.s
index ffe1918399a7cc..b47177ab81cbf3 100644
--- a/llvm/test/MC/AMDGPU/mcexpr_amd_err.s
+++ b/llvm/test/MC/AMDGPU/mcexpr_amd_err.s
@@ -1,14 +1,16 @@
 // RUN: not llvm-mc -triple amdgcn-amd-amdhsa %s 2>&1 | FileCheck --check-prefix=ASM %s
 
-// ASM: error: empty max/or expression
+// ASM: error: empty max expression
+// ASM: error: missing expression
+// ASM: error: empty 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: unexpected token in max 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: unexpected token in or expression
 // ASM: error: missing expression
 
 .set one, 1
@@ -16,11 +18,12 @@
 .set three, 3
 
 .set max_empty, max()
+.set or_empty, or()
 .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 or_expression_one, or(four, five
 
 .set four, 4
 .set five, 5

>From fdd6c61e08f016a574dab94f2f7c382a1aa7abc1 Mon Sep 17 00:00:00 2001
From: Janek van Oirschot <janek.vanoirschot at amd.com>
Date: Mon, 26 Feb 2024 15:33:04 +0000
Subject: [PATCH 04/10] Feedback, add line numbers to test.

---
 .../AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp      |  2 +-
 llvm/test/MC/AMDGPU/mcexpr_amd_err.s          | 24 +++++++++----------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
index 5bd188aae975f1..9a7005541998d3 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
@@ -46,7 +46,7 @@ void AMDGPUVariadicMCExpr::printImpl(raw_ostream &OS,
     if ((It + 1) != Args.end())
       OS << ", ";
   }
-  OS << ")";
+  OS << ')';
 }
 
 bool AMDGPUVariadicMCExpr::evaluateAsRelocatableImpl(
diff --git a/llvm/test/MC/AMDGPU/mcexpr_amd_err.s b/llvm/test/MC/AMDGPU/mcexpr_amd_err.s
index b47177ab81cbf3..6425da676dcfee 100644
--- a/llvm/test/MC/AMDGPU/mcexpr_amd_err.s
+++ b/llvm/test/MC/AMDGPU/mcexpr_amd_err.s
@@ -1,17 +1,17 @@
 // RUN: not llvm-mc -triple amdgcn-amd-amdhsa %s 2>&1 | FileCheck --check-prefix=ASM %s
 
-// ASM: error: empty max expression
-// ASM: error: missing expression
-// ASM: error: empty or expression
-// ASM: error: missing expression
-// ASM: error: unknown token in expression
-// ASM: error: missing expression
-// ASM: error: unexpected token in max expression
-// ASM: error: missing expression
-// ASM: error: unknown token in expression
-// ASM: error: missing expression
-// ASM: error: unexpected token in or expression
-// ASM: error: missing expression
+// ASM: 20:22: error: empty max expression
+// ASM: 20:22: error: missing expression
+// ASM: 21:20: error: empty or expression
+// ASM: 21:20: error: missing expression
+// ASM: 23:29: error: unknown token in expression
+// ASM: 23:29: error: missing expression
+// ASM: 24:32: error: unexpected token in max expression
+// ASM: 24:32: error: missing expression
+// ASM: 25:42: error: unknown token in expression
+// ASM: 25:42: error: missing expression
+// ASM: 26:38: error: unexpected token in or expression
+// ASM: 26:38: error: missing expression
 
 .set one, 1
 .set two, 2

>From 5df4a047abf17e7a09c2176a69e3c273d22eeb3c Mon Sep 17 00:00:00 2001
From: Janek van Oirschot <janek.vanoirschot at amd.com>
Date: Thu, 29 Feb 2024 19:15:40 +0000
Subject: [PATCH 05/10] Feedback, add check for number of commas with tests

---
 .../AMDGPU/AsmParser/AMDGPUAsmParser.cpp      | 11 ++++-
 .../AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp      | 27 ++++++------
 llvm/test/MC/AMDGPU/mcexpr_amd_err.s          | 41 +++++++++++++------
 3 files changed, 52 insertions(+), 27 deletions(-)

diff --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index 18527e103a66af..df99725a0ad4a2 100644
--- a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -8296,6 +8296,7 @@ bool AMDGPUAsmParser::parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) {
 
     if (VK != AGVK::AGVK_None && peekToken().is(AsmToken::LParen)) {
       SmallVector<const MCExpr *, 4> Exprs;
+      uint64_t CommaCount = 0;
       lex(); // Eat 'max'/'or'
       lex(); // Eat '('
       while (true) {
@@ -8305,6 +8306,11 @@ bool AMDGPUAsmParser::parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) {
                   "empty " + Twine(TokenId) + " expression");
             return true;
           }
+          if (CommaCount + 1 != Exprs.size()) {
+            Error(getToken().getLoc(),
+                  "mismatch of commas in " + Twine(TokenId) + " expression");
+            return true;
+          }
           Res = AMDGPUVariadicMCExpr::create(VK, Exprs, getContext());
           return false;
         }
@@ -8312,7 +8318,10 @@ bool AMDGPUAsmParser::parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) {
         if (getParser().parseExpression(Expr, EndLoc))
           return true;
         Exprs.push_back(Expr);
-        if (!trySkipToken(AsmToken::Comma) && !isToken(AsmToken::RParen)) {
+        bool LastTokenWasComma = trySkipToken(AsmToken::Comma);
+        if (LastTokenWasComma)
+          CommaCount++;
+        if (!LastTokenWasComma && !isToken(AsmToken::RParen)) {
           Error(getToken().getLoc(),
                 "unexpected token in " + Twine(TokenId) + " expression");
           return true;
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
index 9a7005541998d3..bd753ff0e5a113 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
@@ -49,20 +49,21 @@ void AMDGPUVariadicMCExpr::printImpl(raw_ostream &OS,
   OS << ')';
 }
 
+static int64_t Op(AMDGPUVariadicMCExpr::AMDGPUVariadicKind Kind, int64_t Arg1,
+                  int64_t Arg2) {
+  switch (Kind) {
+  default:
+    llvm_unreachable("Unknown AMDGPUVariadicMCExpr kind.");
+  case AMDGPUVariadicMCExpr::AMDGPUVariadicKind::AGVK_Max:
+    return std::max(Arg1, Arg2);
+  case AMDGPUVariadicMCExpr::AMDGPUVariadicKind::AGVK_Or:
+    return Arg1 || Arg2;
+  }
+}
+
 bool AMDGPUVariadicMCExpr::evaluateAsRelocatableImpl(
     MCValue &Res, const MCAsmLayout *Layout, const MCFixup *Fixup) const {
-  std::optional<int64_t> Total = {};
-
-  auto Op = [this](int64_t Arg1, int64_t Arg2) -> int64_t {
-    switch (Kind) {
-    default:
-      llvm_unreachable("Unknown AMDGPUVariadicMCExpr kind.");
-    case AGVK_Max:
-      return std::max(Arg1, Arg2);
-    case AGVK_Or:
-      return Arg1 || Arg2;
-    }
-  };
+  std::optional<int64_t> Total;
 
   for (const MCExpr *Arg : Args) {
     MCValue ArgRes;
@@ -72,7 +73,7 @@ bool AMDGPUVariadicMCExpr::evaluateAsRelocatableImpl(
 
     if (!Total.has_value())
       Total = ArgRes.getConstant();
-    Total = Op(*Total, ArgRes.getConstant());
+    Total = Op(Kind, *Total, ArgRes.getConstant());
   }
 
   Res = MCValue::get(*Total);
diff --git a/llvm/test/MC/AMDGPU/mcexpr_amd_err.s b/llvm/test/MC/AMDGPU/mcexpr_amd_err.s
index 6425da676dcfee..e33d8f3e877a19 100644
--- a/llvm/test/MC/AMDGPU/mcexpr_amd_err.s
+++ b/llvm/test/MC/AMDGPU/mcexpr_amd_err.s
@@ -1,29 +1,44 @@
 // RUN: not llvm-mc -triple amdgcn-amd-amdhsa %s 2>&1 | FileCheck --check-prefix=ASM %s
 
-// ASM: 20:22: error: empty max expression
-// ASM: 20:22: error: missing expression
-// ASM: 21:20: error: empty or expression
-// ASM: 21:20: error: missing expression
-// ASM: 23:29: error: unknown token in expression
-// ASM: 23:29: error: missing expression
-// ASM: 24:32: error: unexpected token in max expression
-// ASM: 24:32: error: missing expression
-// ASM: 25:42: error: unknown token in expression
-// ASM: 25:42: error: missing expression
-// ASM: 26:38: error: unexpected token in or expression
-// ASM: 26:38: error: missing expression
-
 .set one, 1
 .set two, 2
 .set three, 3
 
 .set max_empty, max()
+// ASM: :[[@LINE-1]]:{{[0-9]+}}: error: empty max expression
+// ASM: :[[@LINE-2]]:{{[0-9]+}}: error: missing expression
+
 .set or_empty, or()
+// ASM: :[[@LINE-1]]:{{[0-9]+}}: error: empty or expression
+// ASM: :[[@LINE-2]]:{{[0-9]+}}: error: missing expression
+
 .set max_post_aux_comma, max(one,)
+// ASM: :[[@LINE-1]]:{{[0-9]+}}: error: mismatch of commas in max expression
+// ASM: :[[@LINE-2]]:{{[0-9]+}}: error: missing expression
+
 .set max_pre_aux_comma, max(,one)
+// asm: :[[@line-1]]:{{[0-9]+}}: error: unknown token in expression
+// ASM: :[[@LINE-2]]:{{[0-9]+}}: error: missing expression
+
+.set max_double_comma, max(one,, two)
+// ASM: :[[@LINE-1]]:{{[0-9]+}}: error: unknown token in expression
+// ASM: :[[@LINE-2]]:{{[0-9]+}}: error: missing expression
+
+.set max_no_comma, max(one two)
+// ASM: :[[@LINE-1]]:{{[0-9]+}}: error: unexpected token in max expression
+// ASM: :[[@LINE-2]]:{{[0-9]+}}: error: missing expression
+
 .set max_missing_paren, max(two
+// ASM: :[[@LINE-1]]:{{[0-9]+}}: error: unexpected token in max expression
+// ASM: :[[@LINE-2]]:{{[0-9]+}}: error: missing expression
+
 .set max_expression_one, max(three, four,
+// ASM: :[[@LINE-1]]:{{[0-9]+}}: error: unknown token in expression
+// ASM: :[[@LINE-2]]:{{[0-9]+}}: error: missing expression
+
 .set or_expression_one, or(four, five
+// ASM: :[[@LINE-1]]:{{[0-9]+}}: error: unexpected token in or expression
+// ASM: :[[@LINE-2]]:{{[0-9]+}}: error: missing expression
 
 .set four, 4
 .set five, 5

>From 67b279e1022bfd716d80aa685bfd3343fcdf0daa Mon Sep 17 00:00:00 2001
From: Janek van Oirschot <janek.vanoirschot at amd.com>
Date: Fri, 1 Mar 2024 14:40:27 +0000
Subject: [PATCH 06/10] Feedback

---
 .../AMDGPU/AsmParser/AMDGPUAsmParser.cpp      |  2 +-
 .../AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp      | 18 ++++++++--------
 .../Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h | 21 ++++++++++---------
 llvm/test/MC/AMDGPU/mcexpr_amd_err.s          |  9 ++++++++
 4 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index df99725a0ad4a2..16a5d6879ce777 100644
--- a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -8285,7 +8285,7 @@ void AMDGPUAsmParser::onBeginOfFile() {
 ///           max(expr, ...)
 ///
 bool AMDGPUAsmParser::parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) {
-  using AGVK = AMDGPUVariadicMCExpr::AMDGPUVariadicKind;
+  using AGVK = AMDGPUVariadicMCExpr::VariadicKind;
 
   if (isToken(AsmToken::Identifier)) {
     StringRef TokenId = getTokenStr();
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
index bd753ff0e5a113..2602315f3ecf1a 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
@@ -18,15 +18,15 @@
 using namespace llvm;
 
 const AMDGPUVariadicMCExpr *
-AMDGPUVariadicMCExpr::create(AMDGPUVariadicKind Kind,
-                             ArrayRef<const MCExpr *> Args, MCContext &Ctx) {
+AMDGPUVariadicMCExpr::create(VariadicKind Kind, ArrayRef<const MCExpr *> Args,
+                             MCContext &Ctx) {
   return new (Ctx) AMDGPUVariadicMCExpr(Kind, Args);
 }
 
-const MCExpr *AMDGPUVariadicMCExpr::GetSubExpr(size_t index) const {
-  assert(index < Args.size() &&
+const MCExpr *AMDGPUVariadicMCExpr::getSubExpr(size_t Index) const {
+  assert(Index < Args.size() &&
          "Indexing out of bounds AMDGPUVariadicMCExpr sub-expr");
-  return Args[index];
+  return Args[Index];
 }
 
 void AMDGPUVariadicMCExpr::printImpl(raw_ostream &OS,
@@ -49,14 +49,14 @@ void AMDGPUVariadicMCExpr::printImpl(raw_ostream &OS,
   OS << ')';
 }
 
-static int64_t Op(AMDGPUVariadicMCExpr::AMDGPUVariadicKind Kind, int64_t Arg1,
+static int64_t op(AMDGPUVariadicMCExpr::VariadicKind Kind, int64_t Arg1,
                   int64_t Arg2) {
   switch (Kind) {
   default:
     llvm_unreachable("Unknown AMDGPUVariadicMCExpr kind.");
-  case AMDGPUVariadicMCExpr::AMDGPUVariadicKind::AGVK_Max:
+  case AMDGPUVariadicMCExpr::AGVK_Max:
     return std::max(Arg1, Arg2);
-  case AMDGPUVariadicMCExpr::AMDGPUVariadicKind::AGVK_Or:
+  case AMDGPUVariadicMCExpr::AGVK_Or:
     return Arg1 || Arg2;
   }
 }
@@ -73,7 +73,7 @@ bool AMDGPUVariadicMCExpr::evaluateAsRelocatableImpl(
 
     if (!Total.has_value())
       Total = ArgRes.getConstant();
-    Total = Op(Kind, *Total, ArgRes.getConstant());
+    Total = op(Kind, *Total, ArgRes.getConstant());
   }
 
   Res = MCValue::get(*Total);
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
index 1e39810b0ea924..9e35c5b28a5d92 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
@@ -27,34 +27,35 @@ namespace llvm {
 ///
 class AMDGPUVariadicMCExpr : public MCTargetExpr {
 public:
-  enum AMDGPUVariadicKind { AGVK_None, AGVK_Or, AGVK_Max };
+  enum VariadicKind { AGVK_None, AGVK_Or, AGVK_Max };
 
 private:
-  AMDGPUVariadicKind Kind;
+  VariadicKind Kind;
   SmallVector<const MCExpr *, 2> Args;
 
-  AMDGPUVariadicMCExpr(AMDGPUVariadicKind Kind, ArrayRef<const MCExpr *> Args)
+  AMDGPUVariadicMCExpr(VariadicKind Kind, ArrayRef<const MCExpr *> Args)
       : Kind(Kind), Args(Args) {
     assert(Args.size() >= 1 && "Needs a minimum of one expression.");
+    assert(Kind != AGVK_None &&
+           "Cannot construct AMDGPUVariadicMCExpr of kind none.");
   }
 
 public:
-  static const AMDGPUVariadicMCExpr *create(AMDGPUVariadicKind Kind,
-                                            ArrayRef<const MCExpr *> Args,
-                                            MCContext &Ctx);
+  static const AMDGPUVariadicMCExpr *
+  create(VariadicKind Kind, ArrayRef<const MCExpr *> Args, MCContext &Ctx);
 
   static const AMDGPUVariadicMCExpr *createOr(ArrayRef<const MCExpr *> Args,
                                               MCContext &Ctx) {
-    return create(AMDGPUVariadicKind::AGVK_Or, Args, Ctx);
+    return create(VariadicKind::AGVK_Or, Args, Ctx);
   }
 
   static const AMDGPUVariadicMCExpr *createMax(ArrayRef<const MCExpr *> Args,
                                                MCContext &Ctx) {
-    return create(AMDGPUVariadicKind::AGVK_Max, Args, Ctx);
+    return create(VariadicKind::AGVK_Max, Args, Ctx);
   }
 
-  AMDGPUVariadicKind getKind() const { return Kind; }
-  const MCExpr *GetSubExpr(size_t index) const;
+  VariadicKind 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,
diff --git a/llvm/test/MC/AMDGPU/mcexpr_amd_err.s b/llvm/test/MC/AMDGPU/mcexpr_amd_err.s
index e33d8f3e877a19..ea02e013627218 100644
--- a/llvm/test/MC/AMDGPU/mcexpr_amd_err.s
+++ b/llvm/test/MC/AMDGPU/mcexpr_amd_err.s
@@ -40,5 +40,14 @@
 // ASM: :[[@LINE-1]]:{{[0-9]+}}: error: unexpected token in or expression
 // ASM: :[[@LINE-2]]:{{[0-9]+}}: error: missing expression
 
+.set max_no_lparen, max four, five)
+// ASM: :[[@LINE-1]]:{{[0-9]+}}: error: expected newline
+
+.set max_no_paren, max one, two, three
+// ASM: :[[@LINE-1]]:{{[0-9]+}}: error: expected newline
+
+.set max_rparen_only, max)
+// ASM: :[[@LINE-1]]:{{[0-9]+}}: error: expected newline
+
 .set four, 4
 .set five, 5

>From 9f8d4af23a7aef8042ca4deb29f58a4d737078b8 Mon Sep 17 00:00:00 2001
From: Janek van Oirschot <janek.vanoirschot at amd.com>
Date: Tue, 5 Mar 2024 12:04:22 +0000
Subject: [PATCH 07/10] Feedback, change mcexpr from logical to bitwise or,
 documentation on mcexpr in AMDGPUUsage

---
 llvm/docs/AMDGPUUsage.rst                     | 18 +++++++++++++++++
 .../AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp      |  2 +-
 .../Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h |  2 +-
 llvm/test/MC/AMDGPU/mcexpr_amd.s              | 20 +++++++++----------
 4 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/llvm/docs/AMDGPUUsage.rst b/llvm/docs/AMDGPUUsage.rst
index 7f39f69cae60db..6527cfb36c9299 100644
--- a/llvm/docs/AMDGPUUsage.rst
+++ b/llvm/docs/AMDGPUUsage.rst
@@ -1534,6 +1534,24 @@ The AMDGPU backend supports the following calling conventions:
 
      =============================== ==========================================================
 
+AMDGPU MCExpr
+-------------
+
+As part of the AMDGPU MC layer, AMDGPU provides the following target specific
+``MCExpr``\s.
+
+  .. table:: AMDGPU MCExpr types:
+     :name: amdgpu-mcexpr-table
+
+     =================== ======================================================================
+     AMDGPU MCExpr       Description
+     =================== ======================================================================
+     ``max(arg, ...)``   Variadic operation that returns maximum value of all its arguments.
+
+     ``or(arg, ...)``    Variadic operation that returns the bitwise-or result of all its
+                         arguments.
+
+     =================== ======================================================================
 
 .. _amdgpu-elf-code-object:
 
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
index 2602315f3ecf1a..0659937e06a2b4 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
@@ -57,7 +57,7 @@ static int64_t op(AMDGPUVariadicMCExpr::VariadicKind Kind, int64_t Arg1,
   case AMDGPUVariadicMCExpr::AGVK_Max:
     return std::max(Arg1, Arg2);
   case AMDGPUVariadicMCExpr::AGVK_Or:
-    return Arg1 || Arg2;
+    return Arg1 | Arg2;
   }
 }
 
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
index 9e35c5b28a5d92..9e8452f2c624d9 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
@@ -19,7 +19,7 @@ namespace llvm {
 ///
 /// Takes in a minimum of 1 argument to be used with an operation. The supported
 /// operations are:
-///   - (logic) or
+///   - (bitwise) or
 ///   - max
 ///
 /// \note If the 'or'/'max' operations are provided only a single argument, the
diff --git a/llvm/test/MC/AMDGPU/mcexpr_amd.s b/llvm/test/MC/AMDGPU/mcexpr_amd.s
index 3e3d6057318e54..a9639c3acc305a 100644
--- a/llvm/test/MC/AMDGPU/mcexpr_amd.s
+++ b/llvm/test/MC/AMDGPU/mcexpr_amd.s
@@ -29,16 +29,16 @@
 // OBJDUMP-NEXT: 8000000000000000 l       *ABS*  0000000000000000 max_expr_one_min
 // OBJDUMP-NEXT: 0000000000000003 l       *ABS*  0000000000000000 max_expr_two_min
 // OBJDUMP-NEXT: 0000000000989680 l       *ABS*  0000000000000000 max_expr_three_min
-// OBJDUMP-NEXT: 0000000000000001 l       *ABS*  0000000000000000 or_expression_all
-// OBJDUMP-NEXT: 0000000000000001 l       *ABS*  0000000000000000 or_expression_two
+// OBJDUMP-NEXT: 0000000000000007 l       *ABS*  0000000000000000 or_expression_all
+// OBJDUMP-NEXT: 0000000000000003 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: 000000000000000f 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_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
+// OBJDUMP-NEXT: 0000000000000003 l       *ABS*  0000000000000000 or_with_subexpr
+// OBJDUMP-NEXT: 0000000000000008 l       *ABS*  0000000000000000 or_as_subexpr
+// OBJDUMP-NEXT: 0000000000000007 l       *ABS*  0000000000000000 or_recursive_subexpr
 
 // ASM: .set zero, 0
 // ASM: .set one, 1
@@ -98,9 +98,9 @@
 .set max_expr_three_min, max(i64_min, three, 10000000)
 
 // ASM: .set or_expression_all, or(1, 2, five, 3, four)
-// ASM: .set or_expression_two, 1
+// ASM: .set or_expression_two, 3
 // ASM: .set or_expression_one, 1
-// ASM: .set or_literals, 1
+// ASM: .set or_literals, 15
 // ASM: .set or_false, 0
 // ASM: .set or_with_or_sym, or(or, 4, 3, 1, 2)
 
@@ -111,7 +111,7 @@
 .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_with_subexpr, 3
 // ASM: .set or_as_subexpr, 1+(or(4, 3, five))
 // ASM: .set or_recursive_subexpr, or(or(1, four), 3, or_expression_all)
 

>From ade9a397d03352fcb719adfbce74f8402c506102 Mon Sep 17 00:00:00 2001
From: Janek van Oirschot <janek.vanoirschot at amd.com>
Date: Wed, 6 Mar 2024 16:23:34 +0000
Subject: [PATCH 08/10] Feedback, documentation modifications

---
 llvm/docs/AMDGPUUsage.rst | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/llvm/docs/AMDGPUUsage.rst b/llvm/docs/AMDGPUUsage.rst
index 6527cfb36c9299..c7cb06ffbef1e0 100644
--- a/llvm/docs/AMDGPUUsage.rst
+++ b/llvm/docs/AMDGPUUsage.rst
@@ -1543,15 +1543,16 @@ As part of the AMDGPU MC layer, AMDGPU provides the following target specific
   .. table:: AMDGPU MCExpr types:
      :name: amdgpu-mcexpr-table
 
-     =================== ======================================================================
-     AMDGPU MCExpr       Description
-     =================== ======================================================================
-     ``max(arg, ...)``   Variadic operation that returns maximum value of all its arguments.
+     =================== ================= ========================================================
+     MCExpr              Operands          Return value
+     =================== ================= ========================================================
+     ``max(arg, ...)``   1 or more         Variadic signed operation that returns the maximum
+                                           value of all its arguments.
 
-     ``or(arg, ...)``    Variadic operation that returns the bitwise-or result of all its
-                         arguments.
+     ``or(arg, ...)``    1 or more         Variadic signed operation that returns the bitwise-or
+                                           result of all its arguments.
 
-     =================== ======================================================================
+     =================== ================= ========================================================
 
 .. _amdgpu-elf-code-object:
 

>From 6e8f701094e0cb106cf3a10e8e8afb22172a9008 Mon Sep 17 00:00:00 2001
From: Janek van Oirschot <janek.vanoirschot at amd.com>
Date: Fri, 8 Mar 2024 20:25:07 +0000
Subject: [PATCH 09/10] Prevent asan memory leaks that occur through
 SmallVector's heap allocating grow

---
 llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp | 8 +++++++-
 llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h   | 3 +--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
index 0659937e06a2b4..f0fa014547f1af 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
@@ -20,7 +20,13 @@ using namespace llvm;
 const AMDGPUVariadicMCExpr *
 AMDGPUVariadicMCExpr::create(VariadicKind Kind, ArrayRef<const MCExpr *> Args,
                              MCContext &Ctx) {
-  return new (Ctx) AMDGPUVariadicMCExpr(Kind, Args);
+  // Storage for the argument's 'const MCExpr*' allocated through MCContext new placement which means that AMDGPUVariadicMCExpr objects and all of its contents will now be allocated through MCContext new placement.
+  //
+  // Will result in an asan failure if allocated on the heap (e.g., through SmallVector's grow).
+  const MCExpr **CtxArgs = new (Ctx) const MCExpr*[Args.size()];
+  for (size_t i = 0; i < Args.size(); ++i)
+    CtxArgs[i] = Args[i];
+  return new (Ctx) AMDGPUVariadicMCExpr(Kind, ArrayRef(CtxArgs, Args.size()));
 }
 
 const MCExpr *AMDGPUVariadicMCExpr::getSubExpr(size_t Index) const {
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
index 9e8452f2c624d9..cb28556e8be537 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
@@ -10,7 +10,6 @@
 #define LLVM_LIB_TARGET_AMDGPU_MCTARGETDESC_AMDGPUMCEXPR_H
 
 #include "llvm/ADT/ArrayRef.h"
-#include "llvm/ADT/SmallVector.h"
 #include "llvm/MC/MCExpr.h"
 
 namespace llvm {
@@ -31,7 +30,7 @@ class AMDGPUVariadicMCExpr : public MCTargetExpr {
 
 private:
   VariadicKind Kind;
-  SmallVector<const MCExpr *, 2> Args;
+  ArrayRef<const MCExpr *> Args;
 
   AMDGPUVariadicMCExpr(VariadicKind Kind, ArrayRef<const MCExpr *> Args)
       : Kind(Kind), Args(Args) {

>From 23bc4bbbc010175df1c0de0ac43e24efa10a9c64 Mon Sep 17 00:00:00 2001
From: Janek van Oirschot <janek.vanoirschot at amd.com>
Date: Fri, 8 Mar 2024 20:40:55 +0000
Subject: [PATCH 10/10] Code formatting

---
 llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
index f0fa014547f1af..5266a9918c2838 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
@@ -20,10 +20,13 @@ using namespace llvm;
 const AMDGPUVariadicMCExpr *
 AMDGPUVariadicMCExpr::create(VariadicKind Kind, ArrayRef<const MCExpr *> Args,
                              MCContext &Ctx) {
-  // Storage for the argument's 'const MCExpr*' allocated through MCContext new placement which means that AMDGPUVariadicMCExpr objects and all of its contents will now be allocated through MCContext new placement.
+  // Storage for the argument's 'const MCExpr*' allocated through MCContext new
+  // placement which means that AMDGPUVariadicMCExpr objects and all of its
+  // contents will now be allocated through MCContext new placement.
   //
-  // Will result in an asan failure if allocated on the heap (e.g., through SmallVector's grow).
-  const MCExpr **CtxArgs = new (Ctx) const MCExpr*[Args.size()];
+  // Will result in an asan failure if allocated on the heap (e.g., through
+  // SmallVector's grow).
+  const MCExpr **CtxArgs = new (Ctx) const MCExpr *[Args.size()];
   for (size_t i = 0; i < Args.size(); ++i)
     CtxArgs[i] = Args[i];
   return new (Ctx) AMDGPUVariadicMCExpr(Kind, ArrayRef(CtxArgs, Args.size()));



More information about the llvm-commits mailing list