[llvm] [WebAssembly] Add path to PIC mode for wasm tables (PR #67545)

Paulo Matos via llvm-commits llvm-commits at lists.llvm.org
Sun Oct 1 23:35:54 PDT 2023


https://github.com/pmatos updated https://github.com/llvm/llvm-project/pull/67545

>From 035945f74b1abca5d2f679114d6ba10151793860 Mon Sep 17 00:00:00 2001
From: Paulo Matos <pmatos at igalia.com>
Date: Wed, 27 Sep 2023 13:50:39 +0200
Subject: [PATCH 1/6] [WebAssembly] Add path to PIC mode for wasm tables

Currently tables cannot be shared between compilation units, therefore
no special treatment is needed for tables.

Fixes #65191
---
 .../Target/WebAssembly/WebAssemblyISelLowering.cpp   | 10 ++++++++--
 llvm/test/MC/WebAssembly/reloc-pic.s                 | 12 ++++++++++--
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
index fd154a90edef1d9..b67615d82d04a54 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -1724,8 +1724,14 @@ SDValue WebAssemblyTargetLowering::LowerGlobalAddress(SDValue Op,
     fail(DL, DAG, "Invalid address space for WebAssembly target");
 
   unsigned OperandFlags = 0;
-  if (isPositionIndependent()) {
-    const GlobalValue *GV = GA->getGlobal();
+  const GlobalValue *GV = GA->getGlobal();
+  // is PIC but not a WebAssembly table.
+  // Since WebAssembly tables cannot yet be shared accross modules, we don't need special
+  // treatment for tables in PIC mode.
+  if (isPositionIndependent() && 
+      !(GV->getValueType()->isArrayTy() && 
+        WebAssembly::isWebAssemblyReferenceType(GV->getValueType()->getArrayElementType()))) {
+    
     if (getTargetMachine().shouldAssumeDSOLocal(*GV->getParent(), GV)) {
       MachineFunction &MF = DAG.getMachineFunction();
       MVT PtrVT = getPointerTy(MF.getDataLayout());
diff --git a/llvm/test/MC/WebAssembly/reloc-pic.s b/llvm/test/MC/WebAssembly/reloc-pic.s
index dd5d315cc2ce03f..1758820da93479a 100644
--- a/llvm/test/MC/WebAssembly/reloc-pic.s
+++ b/llvm/test/MC/WebAssembly/reloc-pic.s
@@ -1,5 +1,5 @@
-# RUN: llvm-mc -triple=wasm32-unknown-unknown -filetype=obj < %s | obj2yaml | FileCheck %s
-# RUN: llvm-mc -triple=wasm32-unknown-unknown -mattr=+reference-types -filetype=obj < %s | obj2yaml | FileCheck --check-prefix=REF %s
+# RUN: sed -e '/^REF-/d' %s | llvm-mc -triple=wasm32-unknown-unknown -filetype=obj | obj2yaml | FileCheck %s
+# RUN: sed -e 's/^REF-//g' %s | llvm-mc -triple=wasm32-unknown-unknown -mattr=+reference-types -filetype=obj | obj2yaml | FileCheck --check-prefix=REF %s
 
 # Verify that @GOT relocation entryes result in R_WASM_GLOBAL_INDEX_LEB against
 # against the corrsponding function or data symbol and that the corresponding
@@ -50,6 +50,9 @@ hidden_func:
 #.hidden hidden_data
 .size default_data, 4
 
+REF-mytable:
+REF-.tabletype mytable, externref
+
 # CHECK:      --- !WASM
 # CHECK-NEXT: FileHeader:
 # CHECK-NEXT:   Version:         0x1
@@ -209,6 +212,11 @@ hidden_func:
 # CHECK-NEXT:         Function:        5
 # REF:              - Index:           10
 # REF-NEXT:           Kind:            TABLE
+# REF-NEXT:           Name:            mytable
+# REF-NEXT:           Flags:           [ UNDEFINED ]
+# REF-NEXT:           Table:           1
+# REF-NEXT:         - Index:           11
+# REF-NEXT:           Kind:            TABLE
 # REF-NEXT:           Name:            __indirect_function_table
 # REF-NEXT:           Flags:           [ UNDEFINED, NO_STRIP ]
 # REF-NEXT:           Table:           0

>From 9f842f1347c07a0a2b1a1f57ede5aed695757366 Mon Sep 17 00:00:00 2001
From: Paulo Matos <pmatos at igalia.com>
Date: Thu, 28 Sep 2023 15:31:54 +0200
Subject: [PATCH 2/6] Add utility predicate for wasm table types

---
 .../lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h | 6 ++++++
 llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp     | 5 +----
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h b/llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h
index d59fcb3c1b59670..4c6cc8a4fe87b6a 100644
--- a/llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h
+++ b/llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h
@@ -43,6 +43,12 @@ inline bool isWebAssemblyReferenceType(const Type *Ty) {
   return isWebAssemblyExternrefType(Ty) || isWebAssemblyFuncrefType(Ty);
 }
 
+/// Return true if the table represents a WebAssembly table type.
+inline bool isWebAssemblyTableType(const Type *Ty) {
+  return Ty->isArrayTy() && 
+     isWebAssemblyReferenceType(Ty->getArrayElementType());
+}
+
 // Convert StringRef to ValType / HealType / BlockType
 
 MVT parseMVT(StringRef Type);
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
index b67615d82d04a54..9ebb6d1ea5986c7 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -1728,10 +1728,7 @@ SDValue WebAssemblyTargetLowering::LowerGlobalAddress(SDValue Op,
   // is PIC but not a WebAssembly table.
   // Since WebAssembly tables cannot yet be shared accross modules, we don't need special
   // treatment for tables in PIC mode.
-  if (isPositionIndependent() && 
-      !(GV->getValueType()->isArrayTy() && 
-        WebAssembly::isWebAssemblyReferenceType(GV->getValueType()->getArrayElementType()))) {
-    
+  if (isPositionIndependent() && !WebAssembly::isWebAssemblyTableType(GV->getValueType())) {
     if (getTargetMachine().shouldAssumeDSOLocal(*GV->getParent(), GV)) {
       MachineFunction &MF = DAG.getMachineFunction();
       MVT PtrVT = getPointerTy(MF.getDataLayout());

>From 17a6de8d89cb499abe51c882e074922f2f8fc5b5 Mon Sep 17 00:00:00 2001
From: Paulo Matos <pmatos at igalia.com>
Date: Thu, 28 Sep 2023 15:32:07 +0200
Subject: [PATCH 3/6] Run clang format.

---
 .../Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h    | 4 ++--
 llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp    | 7 ++++---
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h b/llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h
index 4c6cc8a4fe87b6a..ce580d57ddc7f7a 100644
--- a/llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h
+++ b/llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h
@@ -45,8 +45,8 @@ inline bool isWebAssemblyReferenceType(const Type *Ty) {
 
 /// Return true if the table represents a WebAssembly table type.
 inline bool isWebAssemblyTableType(const Type *Ty) {
-  return Ty->isArrayTy() && 
-     isWebAssemblyReferenceType(Ty->getArrayElementType());
+  return Ty->isArrayTy() &&
+         isWebAssemblyReferenceType(Ty->getArrayElementType());
 }
 
 // Convert StringRef to ValType / HealType / BlockType
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
index 9ebb6d1ea5986c7..33848079d4b8d5f 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -1726,9 +1726,10 @@ SDValue WebAssemblyTargetLowering::LowerGlobalAddress(SDValue Op,
   unsigned OperandFlags = 0;
   const GlobalValue *GV = GA->getGlobal();
   // is PIC but not a WebAssembly table.
-  // Since WebAssembly tables cannot yet be shared accross modules, we don't need special
-  // treatment for tables in PIC mode.
-  if (isPositionIndependent() && !WebAssembly::isWebAssemblyTableType(GV->getValueType())) {
+  // Since WebAssembly tables cannot yet be shared accross modules, we don't
+  // need special treatment for tables in PIC mode.
+  if (isPositionIndependent() &&
+      !WebAssembly::isWebAssemblyTableType(GV->getValueType())) {
     if (getTargetMachine().shouldAssumeDSOLocal(*GV->getParent(), GV)) {
       MachineFunction &MF = DAG.getMachineFunction();
       MVT PtrVT = getPointerTy(MF.getDataLayout());

>From 61d83f77eacfd133cf692da862a8add5aba8304e Mon Sep 17 00:00:00 2001
From: Paulo Matos <pmatos at igalia.com>
Date: Thu, 28 Sep 2023 15:33:17 +0200
Subject: [PATCH 4/6] Use predicate elsewhere to simplify code

---
 llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.cpp b/llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.cpp
index 4a329bef4681995..86fb99cc98a9055 100644
--- a/llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.cpp
+++ b/llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.cpp
@@ -71,8 +71,7 @@ void WebAssembly::wasmSymbolSetType(MCSymbolWasm *Sym, const Type *GlobalVT,
   // that is a reference type.
   wasm::ValType ValTy;
   bool IsTable = false;
-  if (GlobalVT->isArrayTy() && WebAssembly::isWebAssemblyReferenceType(
-                                   GlobalVT->getArrayElementType())) {
+  if (WebAssembly::isWebAssemblyTableType(GlobalVT)) {
     IsTable = true;
     const Type *ElTy = GlobalVT->getArrayElementType();
     if (WebAssembly::isWebAssemblyExternrefType(ElTy))

>From 0730f2b1998db30eacb72729d3840aa8151abaf7 Mon Sep 17 00:00:00 2001
From: Paulo Matos <pmatos at igalia.com>
Date: Fri, 29 Sep 2023 10:33:02 +0200
Subject: [PATCH 5/6] Check for pointerty in reference types predicate.

---
 llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h b/llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h
index ce580d57ddc7f7a..220fe7b4eaf05e8 100644
--- a/llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h
+++ b/llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h
@@ -28,13 +28,13 @@ namespace WebAssembly {
 
 /// Return true if this is a WebAssembly Externref Type.
 inline bool isWebAssemblyExternrefType(const Type *Ty) {
-  return Ty->getPointerAddressSpace() ==
+  return Ty->isPointerTy() && Ty->getPointerAddressSpace() ==
          WebAssembly::WasmAddressSpace::WASM_ADDRESS_SPACE_EXTERNREF;
 }
 
 /// Return true if this is a WebAssembly Funcref Type.
 inline bool isWebAssemblyFuncrefType(const Type *Ty) {
-  return Ty->getPointerAddressSpace() ==
+  return Ty->isPointerTy() && Ty->getPointerAddressSpace() ==
          WebAssembly::WasmAddressSpace::WASM_ADDRESS_SPACE_FUNCREF;
 }
 

>From 93f749ea5d5cb2d6010a575c8f270ab9698d6a43 Mon Sep 17 00:00:00 2001
From: Paulo Matos <pmatos at igalia.com>
Date: Mon, 2 Oct 2023 08:35:39 +0200
Subject: [PATCH 6/6] Remove unnecessary comment.

---
 llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
index 33848079d4b8d5f..61cfcdc914cdb93 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -1725,7 +1725,6 @@ SDValue WebAssemblyTargetLowering::LowerGlobalAddress(SDValue Op,
 
   unsigned OperandFlags = 0;
   const GlobalValue *GV = GA->getGlobal();
-  // is PIC but not a WebAssembly table.
   // Since WebAssembly tables cannot yet be shared accross modules, we don't
   // need special treatment for tables in PIC mode.
   if (isPositionIndependent() &&



More information about the llvm-commits mailing list