[PATCH] D95425: Implementation of global.get/set for reftypes in LLVM IR

[Andy Wingo via Phabricator via cfe-commits]

wingo added inline comments.



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyInstrTable.td:15
 multiclass TABLE {
+  let mayLoad = 1 in
   defm TABLE_GET_#rt : I<(outs rt:$res), (ins table32_op:$table),

pmatos wrote:
> tlively wrote:
> > wingo wrote:
> > > I think you may need `hasSideEffects = 0` for these annotations to have 
> > > an effect.
> > I would be surprised if this were true!
> Why would this be the case? If I remember correctly, I added `mayLoad` and 
> `mayStore` here so that lowering includes a chain. And this works without the 
> need for `hasSideEffects`. Unless you think this is required for other 
> reaons, but `mayLoad` works with it.
Yeah I misunderstood, I was thinking that `mayLoad` and `mayStore` were a 
subset of the side effect annotations of `hasSideEffects`, which defaults to 1 
effectively (grep for `guessInstructionProperties`).  But no, the side effects 
modelled here are the disjoint bits `mayLoad`, `mayStore`, and 
`hasSideEffects`; `mayRaiseFPException` would appear to be a subset of 
`hasSideEffects`.  Still, may best to explicitly set `hasSideEffects` to 1 for 
`table.get ` and `table.set` just as documentation that they can trap if the 
index is out-of-bounds.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95425/new/

https://reviews.llvm.org/D95425

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

[Andy Wingo via Phabricator via cfe-commits]

wingo updated this revision to Diff 346986.
wingo added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101140/new/

https://reviews.llvm.org/D101140

Files:
  llvm/include/llvm/CodeGen/MIRYamlMapping.h
  llvm/include/llvm/CodeGen/TargetFrameLowering.h
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h
  llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyISD.def
  llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
  llvm/test/CodeGen/WebAssembly/ir-locals.ll

Index: llvm/test/CodeGen/WebAssembly/ir-locals.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals.ll
@@ -0,0 +1,76 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false | FileCheck %s
+
+%i32_cell = type i32 addrspace(1)*
+%i64_cell = type i64 addrspace(1)*
+%f32_cell = type float addrspace(1)*
+%f64_cell = type double addrspace(1)*
+
+define i32 @ir_local_i32(i32 %arg) {
+ ; CHECK-LABEL: ir_local_i32:
+ ; CHECK-NEXT: .functype ir_local_i32 (i32) -> (i32)
+ %retval = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ store i32 %arg, %i32_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ %reloaded = load i32, %i32_cell %retval
+ ; The DAG combiner infers that %reloaded is the same as %arg and
+ ; ultimately causes "local.get 0" to be emitted instead of
+ ; "local.get 1".
+ ; CHECK-NEXT: local.get 0
+ ret i32 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define i64 @ir_local_i64(i64 %arg) {
+ ; CHECK-LABEL: ir_local_i64:
+ ; CHECK-NEXT: .functype ir_local_i64 (i64) -> (i64)
+ %retval = alloca i64, addrspace(1)
+ ; CHECK-NEXT: .local i64
+ store i64 %arg, %i64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ %reloaded = load i64, %i64_cell %retval
+ ; See note in ir_local_i32.
+ ; CHECK-NEXT: local.get 0
+ ret i64 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define float @ir_local_f32(float %arg) {
+ ; CHECK-LABEL: ir_local_f32:
+ ; CHECK-NEXT: .functype ir_local_f32 (f32) -> (f32)
+ %retval = alloca float, addrspace(1)
+ ; CHECK-NEXT: .local f32
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ store float %arg, %f32_cell %retval
+ ; See note in ir_local_i32.
+ ; CHECK-NEXT: local.get 0
+ %reloaded = load float, %f32_cell %retval
+ ; CHECK-NEXT: end_function
+ ret float %reloaded
+}
+
+define double @ir_local_f64(double %arg) {
+ ; CHECK-LABEL: ir_local_f64:
+ ; CHECK-NEXT: .functype ir_local_f64 (f64) -> (f64)
+ %retval = alloca double, addrspace(1)
+ ; CHECK-NEXT: .local f64
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ store double %arg, %f64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ %reloaded = load double, %f64_cell %retval
+ ; CHECK-NEXT: end_function
+ ret double %reloaded
+}
+
+define void @ir_unreferenced_local() {
+ ; CHECK-LABEL: ir_unreferenced_local:
+ ; CHECK-NEXT: .functype ir_unreferenced_local () -> ()
+ %unused = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ ret void
+ ; CHECK-NEXT: end_function
+}
Index: llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
@@ -0,0 +1,22 @@
+; RUN: llc -mtriple=wasm32-unknown-unknown -asm-verbose=false < %s | FileCheck %s --check-prefix=CHECKCG
+; RUN: llc -mtriple=wasm32-unknown-unknown -stop-after=finalize-isel < %s | FileCheck %s --check-prefix=CHECKISEL
+
+%f32_cell = type float addrspace(1)*
+
+; CHECKISEL-LABEL: name: ir_local_f32
+; CHECKISEL:   stack:
+; CHECKISEL:   id: 0, name: retval, type: default, offset: 1, size: 1, alignment: 4,
+; CHECKISEL-NEXT:  stack-id: wasm-local
+
+; CHECKCG-LABEL: ir_local_f32:
+; CHECKCG-NEXT: .functype ir_local_f32 (f32) -> (f32)
+; CHECKCG-NEXT: .local f32
+; CHECKCG-NEXT: local.get 0
+; CHECKCG-NEXT: local.set 1
+
+define float @ir_local_f32(float %arg) {
+ %retval = alloca float, addrspace(1)
+ store float %arg, %f32_cell %retval
+ %reloaded = load float, %f32_cell %retval
+ ret float %reloaded
+}
Index: llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
===
--- llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
+++ llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
@@ -72,6 +72,8 @@
 SDCallSeqEnd<[SDTCisVT<0, iPTR>, SDTCisVT<1, iPTR>]>;
 def SDT_WebAssemblyBrTable: SDTypeProfile<0, -1, [SDTCisPtrTy<0>]>;
 def SDT_WebAssemblyArgument   : SDTypeProfile<1, 1, [SDTCisVT<1, i32>]>;
+def SDT_WebAssemblyLocalGet   : SDTypeP

[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

[Andy Wingo via Phabricator via cfe-commits]

wingo updated this revision to Diff 346990.
wingo marked 4 inline comments as done.
wingo added a comment.

Address feedback


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101140/new/

https://reviews.llvm.org/D101140

Files:
  llvm/include/llvm/CodeGen/MIRYamlMapping.h
  llvm/include/llvm/CodeGen/TargetFrameLowering.h
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h
  llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyISD.def
  llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
  llvm/test/CodeGen/WebAssembly/ir-locals.ll

Index: llvm/test/CodeGen/WebAssembly/ir-locals.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals.ll
@@ -0,0 +1,76 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false | FileCheck %s
+
+%i32_cell = type i32 addrspace(1)*
+%i64_cell = type i64 addrspace(1)*
+%f32_cell = type float addrspace(1)*
+%f64_cell = type double addrspace(1)*
+
+define i32 @ir_local_i32(i32 %arg) {
+ ; CHECK-LABEL: ir_local_i32:
+ ; CHECK-NEXT: .functype ir_local_i32 (i32) -> (i32)
+ %retval = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ store i32 %arg, %i32_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ %reloaded = load i32, %i32_cell %retval
+ ; The DAG combiner infers that %reloaded is the same as %arg and
+ ; ultimately causes "local.get 0" to be emitted instead of
+ ; "local.get 1".
+ ; CHECK-NEXT: local.get 0
+ ret i32 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define i64 @ir_local_i64(i64 %arg) {
+ ; CHECK-LABEL: ir_local_i64:
+ ; CHECK-NEXT: .functype ir_local_i64 (i64) -> (i64)
+ %retval = alloca i64, addrspace(1)
+ ; CHECK-NEXT: .local i64
+ store i64 %arg, %i64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ %reloaded = load i64, %i64_cell %retval
+ ; See note in ir_local_i32.
+ ; CHECK-NEXT: local.get 0
+ ret i64 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define float @ir_local_f32(float %arg) {
+ ; CHECK-LABEL: ir_local_f32:
+ ; CHECK-NEXT: .functype ir_local_f32 (f32) -> (f32)
+ %retval = alloca float, addrspace(1)
+ ; CHECK-NEXT: .local f32
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ store float %arg, %f32_cell %retval
+ ; See note in ir_local_i32.
+ ; CHECK-NEXT: local.get 0
+ %reloaded = load float, %f32_cell %retval
+ ; CHECK-NEXT: end_function
+ ret float %reloaded
+}
+
+define double @ir_local_f64(double %arg) {
+ ; CHECK-LABEL: ir_local_f64:
+ ; CHECK-NEXT: .functype ir_local_f64 (f64) -> (f64)
+ %retval = alloca double, addrspace(1)
+ ; CHECK-NEXT: .local f64
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ store double %arg, %f64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ %reloaded = load double, %f64_cell %retval
+ ; CHECK-NEXT: end_function
+ ret double %reloaded
+}
+
+define void @ir_unreferenced_local() {
+ ; CHECK-LABEL: ir_unreferenced_local:
+ ; CHECK-NEXT: .functype ir_unreferenced_local () -> ()
+ %unused = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ ret void
+ ; CHECK-NEXT: end_function
+}
Index: llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
@@ -0,0 +1,22 @@
+; RUN: llc -mtriple=wasm32-unknown-unknown -asm-verbose=false < %s | FileCheck %s --check-prefix=CHECKCG
+; RUN: llc -mtriple=wasm32-unknown-unknown -stop-after=finalize-isel < %s | FileCheck %s --check-prefix=CHECKISEL
+
+%f32_cell = type float addrspace(1)*
+
+; CHECKISEL-LABEL: name: ir_local_f32
+; CHECKISEL:   stack:
+; CHECKISEL:   id: 0, name: retval, type: default, offset: 1, size: 1, alignment: 4,
+; CHECKISEL-NEXT:  stack-id: wasm-local
+
+; CHECKCG-LABEL: ir_local_f32:
+; CHECKCG-NEXT: .functype ir_local_f32 (f32) -> (f32)
+; CHECKCG-NEXT: .local f32
+; CHECKCG-NEXT: local.get 0
+; CHECKCG-NEXT: local.set 1
+
+define float @ir_local_f32(float %arg) {
+ %retval = alloca float, addrspace(1)
+ store float %arg, %f32_cell %retval
+ %reloaded = load float, %f32_cell %retval
+ ret float %reloaded
+}
Index: llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
===
--- llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
+++ llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
@@ -72,6 +72,8 @@
 SDCallSeqEnd<[SDTCisVT<0, iPTR>, SDTCisVT<1, iPTR>]>;
 def SDT_WebAssemblyBrTable: SDTypeProfile<0, -1, [SDTCisPtrTy<0>]>;
 def SDT_WebAssemblyArgument   : SDTypeProfile<1, 1, [SDTCisVT<1

[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

[Andy Wingo via Phabricator via cfe-commits]

wingo added inline comments.



Comment at: llvm/include/llvm/CodeGen/MIRYamlMapping.h:351
 IO.enumCase(ID, "scalable-vector", TargetStackID::ScalableVector);
+IO.enumCase(ID, "object", TargetStackID::Object);
 IO.enumCase(ID, "noalloc", TargetStackID::NoAlloc);

tlively wrote:
> Is there a good test to demonstrate this change in?
Done in ir-locals-stackid.ll



Comment at: llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp:34-36
+// runs between then and DAG building time, though, so instead we hoist stack
+// objects lazily when they are first used, and comprehensively after the DAG 
is
+// built via the PreprocessISelDAG hook, called by the

tlively wrote:
> Why do this in two places instead of just once comprehensively in the hook? 
> It would be good to explain that in the comment, too.
👍 



Comment at: llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp:63-64
+  MFI.setObjectOffset(FrameIndex, Local);
+  // Allocate WebAssembly locals for each non-aggregate component of the
+  // allocation.
+  for (EVT ValueVT : ValueVTs)

tlively wrote:
> Can there be aggregate components? What do we do with them?
Hoo, it is a good question.  I think the high-level answer is that in the same 
way as you can allocate an i32 to a local, or an externref to a local, you 
could allocate a struct { i32, externref }.  The leaves of that aggregate would 
be numbered from 0 to N, assigned to consecutive locals of the appropriate 
primitive types, and accesses would compiled down to direct access to the 
members.  Probably the first person who tries this is going to run into some 
interesting cases and I think specifically the local.get index mapping isn't 
quite yet ready.  But that would be the idea.  WDYT?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101140/new/

https://reviews.llvm.org/D101140

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

[Andy Wingo via Phabricator via cfe-commits]

wingo added a comment.

Gentle ping here to @tlively :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101140/new/

https://reviews.llvm.org/D101140

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

[Andy Wingo via Phabricator via cfe-commits]

wingo added inline comments.



Comment at: llvm/test/CodeGen/WebAssembly/ir-locals.ll:17-20
+ ; The DAG combiner infers that %reloaded is the same as %arg and
+ ; ultimately causes "local.get 0" to be emitted instead of
+ ; "local.get 1".
+ ; CHECK-NEXT: local.get 0

tlively wrote:
> It might be good to get the value from an external function call rather than 
> from an argument to prevent this transformation from happening.
Tx for feedback.  Actually it doesn't really matter where the value comes from; 
store-to-load forwarding happens regardless.  If it came from a call it would 
be allocated a local via the explicit locals pass, as a value with more than 
one use.

However I was able to inhibit this transformation with an opaque call, so I'll 
do that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101140/new/

https://reviews.llvm.org/D101140

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

[Andy Wingo via Phabricator via cfe-commits]

wingo updated this revision to Diff 348467.
wingo added a comment.

Adapt test to insert compiler barrier.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101140/new/

https://reviews.llvm.org/D101140

Files:
  llvm/include/llvm/CodeGen/MIRYamlMapping.h
  llvm/include/llvm/CodeGen/TargetFrameLowering.h
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h
  llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyISD.def
  llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
  llvm/test/CodeGen/WebAssembly/ir-locals.ll

Index: llvm/test/CodeGen/WebAssembly/ir-locals.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals.ll
@@ -0,0 +1,87 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false | FileCheck %s
+
+%i32_cell = type i32 addrspace(1)*
+%i64_cell = type i64 addrspace(1)*
+%f32_cell = type float addrspace(1)*
+%f64_cell = type double addrspace(1)*
+
+; We have a set of tests in which we set a local and then reload the
+; local.  If the load immediately follows the set, the DAG combiner will
+; infer that the reloaded value is the same value that was set, which
+; isn't what we want to test.  To inhibit this optimization, we include
+; an opaque call between the store and the load.
+declare void @inhibit_store_to_load_forwarding()
+
+define i32 @ir_local_i32(i32 %arg) {
+ ; CHECK-LABEL: ir_local_i32:
+ ; CHECK-NEXT: .functype ir_local_i32 (i32) -> (i32)
+ %retval = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ store i32 %arg, %i32_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load i32, %i32_cell %retval
+ ; CHECK-NEXT: local.get 1
+ ret i32 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define i64 @ir_local_i64(i64 %arg) {
+ ; CHECK-LABEL: ir_local_i64:
+ ; CHECK-NEXT: .functype ir_local_i64 (i64) -> (i64)
+ %retval = alloca i64, addrspace(1)
+ ; CHECK-NEXT: .local i64
+ store i64 %arg, %i64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load i64, %i64_cell %retval
+ ; See note in ir_local_i32.
+ ; CHECK-NEXT: local.get 1
+ ret i64 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define float @ir_local_f32(float %arg) {
+ ; CHECK-LABEL: ir_local_f32:
+ ; CHECK-NEXT: .functype ir_local_f32 (f32) -> (f32)
+ %retval = alloca float, addrspace(1)
+ ; CHECK-NEXT: .local f32
+ store float %arg, %f32_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load float, %f32_cell %retval
+ ; CHECK-NEXT: local.get 1
+ ; CHECK-NEXT: end_function
+ ret float %reloaded
+}
+
+define double @ir_local_f64(double %arg) {
+ ; CHECK-LABEL: ir_local_f64:
+ ; CHECK-NEXT: .functype ir_local_f64 (f64) -> (f64)
+ %retval = alloca double, addrspace(1)
+ ; CHECK-NEXT: .local f64
+ store double %arg, %f64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load double, %f64_cell %retval
+ ; CHECK-NEXT: local.get 1
+ ; CHECK-NEXT: end_function
+ ret double %reloaded
+}
+
+define void @ir_unreferenced_local() {
+ ; CHECK-LABEL: ir_unreferenced_local:
+ ; CHECK-NEXT: .functype ir_unreferenced_local () -> ()
+ %unused = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ ret void
+ ; CHECK-NEXT: end_function
+}
Index: llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
@@ -0,0 +1,22 @@
+; RUN: llc -mtriple=wasm32-unknown-unknown -asm-verbose=false < %s | FileCheck %s --check-prefix=CHECKCG
+; RUN: llc -mtriple=wasm32-unknown-unknown -stop-after=finalize-isel < %s | FileCheck %s --check-prefix=CHECKISEL
+
+%f32_cell = type float addrspace(1)*
+
+; CHECKISEL-LABEL: name: ir_local_f32
+; CHECKISEL:   stack:
+; CHECKISEL:   id: 0, name: retval, type: default, offset: 1, size: 1, alignment: 4,
+; CHECKISEL-NEXT:  stack-id: wasm-local
+
+; CHECKCG-LABEL: ir_local_f32:
+; CHECKCG-NEXT: .functype ir_local_f32 (f32) -> (f32)
+; CHECKCG-NEXT: .local f32
+; CHECKCG-NEXT: local.get 0
+; CHECKCG-NEXT: local.set 1
+
+define float @ir_local_f32(float %arg

[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

[Andy Wingo via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG00ecf18979e3: [WebAssembly][CodeGen] IR support for 
WebAssembly local variables (authored by wingo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101140/new/

https://reviews.llvm.org/D101140

Files:
  llvm/include/llvm/CodeGen/MIRYamlMapping.h
  llvm/include/llvm/CodeGen/TargetFrameLowering.h
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h
  llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyISD.def
  llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
  llvm/test/CodeGen/WebAssembly/ir-locals.ll

Index: llvm/test/CodeGen/WebAssembly/ir-locals.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals.ll
@@ -0,0 +1,87 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false | FileCheck %s
+
+%i32_cell = type i32 addrspace(1)*
+%i64_cell = type i64 addrspace(1)*
+%f32_cell = type float addrspace(1)*
+%f64_cell = type double addrspace(1)*
+
+; We have a set of tests in which we set a local and then reload the
+; local.  If the load immediately follows the set, the DAG combiner will
+; infer that the reloaded value is the same value that was set, which
+; isn't what we want to test.  To inhibit this optimization, we include
+; an opaque call between the store and the load.
+declare void @inhibit_store_to_load_forwarding()
+
+define i32 @ir_local_i32(i32 %arg) {
+ ; CHECK-LABEL: ir_local_i32:
+ ; CHECK-NEXT: .functype ir_local_i32 (i32) -> (i32)
+ %retval = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ store i32 %arg, %i32_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load i32, %i32_cell %retval
+ ; CHECK-NEXT: local.get 1
+ ret i32 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define i64 @ir_local_i64(i64 %arg) {
+ ; CHECK-LABEL: ir_local_i64:
+ ; CHECK-NEXT: .functype ir_local_i64 (i64) -> (i64)
+ %retval = alloca i64, addrspace(1)
+ ; CHECK-NEXT: .local i64
+ store i64 %arg, %i64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load i64, %i64_cell %retval
+ ; See note in ir_local_i32.
+ ; CHECK-NEXT: local.get 1
+ ret i64 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define float @ir_local_f32(float %arg) {
+ ; CHECK-LABEL: ir_local_f32:
+ ; CHECK-NEXT: .functype ir_local_f32 (f32) -> (f32)
+ %retval = alloca float, addrspace(1)
+ ; CHECK-NEXT: .local f32
+ store float %arg, %f32_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load float, %f32_cell %retval
+ ; CHECK-NEXT: local.get 1
+ ; CHECK-NEXT: end_function
+ ret float %reloaded
+}
+
+define double @ir_local_f64(double %arg) {
+ ; CHECK-LABEL: ir_local_f64:
+ ; CHECK-NEXT: .functype ir_local_f64 (f64) -> (f64)
+ %retval = alloca double, addrspace(1)
+ ; CHECK-NEXT: .local f64
+ store double %arg, %f64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load double, %f64_cell %retval
+ ; CHECK-NEXT: local.get 1
+ ; CHECK-NEXT: end_function
+ ret double %reloaded
+}
+
+define void @ir_unreferenced_local() {
+ ; CHECK-LABEL: ir_unreferenced_local:
+ ; CHECK-NEXT: .functype ir_unreferenced_local () -> ()
+ %unused = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ ret void
+ ; CHECK-NEXT: end_function
+}
Index: llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
@@ -0,0 +1,22 @@
+; RUN: llc -mtriple=wasm32-unknown-unknown -asm-verbose=false < %s | FileCheck %s --check-prefix=CHECKCG
+; RUN: llc -mtriple=wasm32-unknown-unknown -stop-after=finalize-isel < %s | FileCheck %s --check-prefix=CHECKISEL
+
+%f32_cell = type float addrspace(1)*
+
+; CHECKISEL-LABEL: name: ir_local_f32
+; CHECKISEL:   stack:
+; CHECKISEL:   id: 0, name: retval, type: default, offset: 1, size: 1, alignment: 4,
+; CHECKISEL-NEXT:  stack-id: wasm-local
+
+; CHECKCG-LABEL: ir_local_f32:
+; CHECKCG-NEXT: .functype ir_local_f3

[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

[Andy Wingo via Phabricator via cfe-commits]

wingo updated this revision to Diff 348493.
wingo added a comment.
Herald added subscribers: foad, frasercrmck, kerbowa, luismarques, apazos, 
sameer.abuasal, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, 
MartinMosbeck, rogfer01, edward-jones, zzheng, MaskRay, jrtc27, niosHD, 
sabuasal, simoncook, johnrusso, rbar, asb, nhaehnle, jvesely, arsenm.

Fix build for RISC-V and AMDGPU.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101140/new/

https://reviews.llvm.org/D101140

Files:
  llvm/include/llvm/CodeGen/MIRYamlMapping.h
  llvm/include/llvm/CodeGen/TargetFrameLowering.h
  llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
  llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h
  llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyISD.def
  llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
  llvm/test/CodeGen/WebAssembly/ir-locals.ll

Index: llvm/test/CodeGen/WebAssembly/ir-locals.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals.ll
@@ -0,0 +1,87 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false | FileCheck %s
+
+%i32_cell = type i32 addrspace(1)*
+%i64_cell = type i64 addrspace(1)*
+%f32_cell = type float addrspace(1)*
+%f64_cell = type double addrspace(1)*
+
+; We have a set of tests in which we set a local and then reload the
+; local.  If the load immediately follows the set, the DAG combiner will
+; infer that the reloaded value is the same value that was set, which
+; isn't what we want to test.  To inhibit this optimization, we include
+; an opaque call between the store and the load.
+declare void @inhibit_store_to_load_forwarding()
+
+define i32 @ir_local_i32(i32 %arg) {
+ ; CHECK-LABEL: ir_local_i32:
+ ; CHECK-NEXT: .functype ir_local_i32 (i32) -> (i32)
+ %retval = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ store i32 %arg, %i32_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load i32, %i32_cell %retval
+ ; CHECK-NEXT: local.get 1
+ ret i32 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define i64 @ir_local_i64(i64 %arg) {
+ ; CHECK-LABEL: ir_local_i64:
+ ; CHECK-NEXT: .functype ir_local_i64 (i64) -> (i64)
+ %retval = alloca i64, addrspace(1)
+ ; CHECK-NEXT: .local i64
+ store i64 %arg, %i64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load i64, %i64_cell %retval
+ ; See note in ir_local_i32.
+ ; CHECK-NEXT: local.get 1
+ ret i64 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define float @ir_local_f32(float %arg) {
+ ; CHECK-LABEL: ir_local_f32:
+ ; CHECK-NEXT: .functype ir_local_f32 (f32) -> (f32)
+ %retval = alloca float, addrspace(1)
+ ; CHECK-NEXT: .local f32
+ store float %arg, %f32_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load float, %f32_cell %retval
+ ; CHECK-NEXT: local.get 1
+ ; CHECK-NEXT: end_function
+ ret float %reloaded
+}
+
+define double @ir_local_f64(double %arg) {
+ ; CHECK-LABEL: ir_local_f64:
+ ; CHECK-NEXT: .functype ir_local_f64 (f64) -> (f64)
+ %retval = alloca double, addrspace(1)
+ ; CHECK-NEXT: .local f64
+ store double %arg, %f64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load double, %f64_cell %retval
+ ; CHECK-NEXT: local.get 1
+ ; CHECK-NEXT: end_function
+ ret double %reloaded
+}
+
+define void @ir_unreferenced_local() {
+ ; CHECK-LABEL: ir_unreferenced_local:
+ ; CHECK-NEXT: .functype ir_unreferenced_local () -> ()
+ %unused = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ ret void
+ ; CHECK-NEXT: end_function
+}
Index: llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
@@ -0,0 +1,22 @@
+; RUN: llc -mtriple=wasm32-unknown-unknown -asm-verbose=false < %s | FileCheck %s --check-prefix=CHECKCG
+; RUN: llc -mtriple=wasm32-unknown-unknown -stop-after=finalize-isel < %s | FileCheck %s --check-prefix=CHECKISEL
+
+%f32_cell = type float addrspace(1)*
+
+; CHECKISEL-LABEL: name: ir_local_f32
+

[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

[Andy Wingo via Phabricator via cfe-commits]

wingo added a comment.

In D101140#2786777 , @jrtc27 wrote:

> Is it just me or does having a backend-specific type in target-independent 
> code feel wrong? It feels like there should be a space for target-specific 
> TargetStackIDs...

Hoo, good question.  More support for target-specific handling of stack IDs 
would be great.  However in this case the concept is not purely wasm-specific; 
I can imagine other targets that might have a similar treatment of locals (if 
we had a .net target, or a jvm target, or so).  The idea is that there is a 
separate stack consisting of named locals that may not be addressable by 
pointers to main memory.  In earlier drafts of this patch the name was more 
generic ("Object", then "Managed") but you know, our words in this area are 
quite overloaded.  So instead I went with something quite specific (WasmLocal) 
to avoid the general question -- but I do think the concept is not specific, 
even if it doesn't apply to any other target currently in tree.  WDYT?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101140/new/

https://reviews.llvm.org/D101140

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

[Andy Wingo via Phabricator via cfe-commits]

wingo added a comment.

In D101140#2786870 , @jrtc27 wrote:

> In D101140#2786844 , @wingo wrote:
>
>> In D101140#2786777 , @jrtc27 wrote:
>>
>>> Is it just me or does having a backend-specific type in target-independent 
>>> code feel wrong? It feels like there should be a space for target-specific 
>>> TargetStackIDs...
>>
>> Hoo, good question.  More support for target-specific handling of stack IDs 
>> would be great.  However in this case the concept is not purely 
>> wasm-specific; I can imagine other targets that might have a similar 
>> treatment of locals (if we had a .net target, or a jvm target, or so).  The 
>> idea is that there is a separate stack consisting of named locals that may 
>> not be addressable by pointers to main memory.  In earlier drafts of this 
>> patch the name was more generic ("Object", then "Managed") but you know, our 
>> words in this area are quite overloaded.  So instead I went with something 
>> quite specific (WasmLocal) to avoid the general question -- but I do think 
>> the concept is not specific, even if it doesn't apply to any other target 
>> currently in tree.  WDYT?
>
> Well, except all the logic for it is in the backend, only the parser and the 
> definition are in target-independent code, so even if another backend were to 
> reuse it it would have its own completely separate logic for it, and thus 
> there's no benefit to reusing a shared name for the thing over each target 
> defining its own? Or would some of the code in the wasm backend be refactored 
> out into CodeGen?

This is the case for all `TargetStackID` enumerated values except 
`TargetStackID::Default` -- `ScalableVector` and `SGPRSpill` have 0 uses 
outside target backends, though the ScalableVector name is used in RISC-V and 
AMDGPU.  `NoAlloc` has one mention outside the target backends but no real 
logic.  Still, as a reader who hacks neither on RISC-V nor AMDGPU  I find it 
useful that they share the same stack ID definition for scalable vectors -- 
lets me compare things a bit easier.

If there is a change to be made in this area (moving these enumerated values 
elsewhere) it would have to be in a followup I think and would need 
consultation with the other backends that use these values.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101140/new/

https://reviews.llvm.org/D101140

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

[Andy Wingo via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbf35f4af51cd: [WebAssembly][CodeGen] IR support for 
WebAssembly local variables (authored by wingo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101140/new/

https://reviews.llvm.org/D101140

Files:
  llvm/include/llvm/CodeGen/MIRYamlMapping.h
  llvm/include/llvm/CodeGen/TargetFrameLowering.h
  llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
  llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h
  llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyISD.def
  llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
  llvm/test/CodeGen/WebAssembly/ir-locals.ll

Index: llvm/test/CodeGen/WebAssembly/ir-locals.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals.ll
@@ -0,0 +1,87 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false | FileCheck %s
+
+%i32_cell = type i32 addrspace(1)*
+%i64_cell = type i64 addrspace(1)*
+%f32_cell = type float addrspace(1)*
+%f64_cell = type double addrspace(1)*
+
+; We have a set of tests in which we set a local and then reload the
+; local.  If the load immediately follows the set, the DAG combiner will
+; infer that the reloaded value is the same value that was set, which
+; isn't what we want to test.  To inhibit this optimization, we include
+; an opaque call between the store and the load.
+declare void @inhibit_store_to_load_forwarding()
+
+define i32 @ir_local_i32(i32 %arg) {
+ ; CHECK-LABEL: ir_local_i32:
+ ; CHECK-NEXT: .functype ir_local_i32 (i32) -> (i32)
+ %retval = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ store i32 %arg, %i32_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load i32, %i32_cell %retval
+ ; CHECK-NEXT: local.get 1
+ ret i32 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define i64 @ir_local_i64(i64 %arg) {
+ ; CHECK-LABEL: ir_local_i64:
+ ; CHECK-NEXT: .functype ir_local_i64 (i64) -> (i64)
+ %retval = alloca i64, addrspace(1)
+ ; CHECK-NEXT: .local i64
+ store i64 %arg, %i64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load i64, %i64_cell %retval
+ ; See note in ir_local_i32.
+ ; CHECK-NEXT: local.get 1
+ ret i64 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define float @ir_local_f32(float %arg) {
+ ; CHECK-LABEL: ir_local_f32:
+ ; CHECK-NEXT: .functype ir_local_f32 (f32) -> (f32)
+ %retval = alloca float, addrspace(1)
+ ; CHECK-NEXT: .local f32
+ store float %arg, %f32_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load float, %f32_cell %retval
+ ; CHECK-NEXT: local.get 1
+ ; CHECK-NEXT: end_function
+ ret float %reloaded
+}
+
+define double @ir_local_f64(double %arg) {
+ ; CHECK-LABEL: ir_local_f64:
+ ; CHECK-NEXT: .functype ir_local_f64 (f64) -> (f64)
+ %retval = alloca double, addrspace(1)
+ ; CHECK-NEXT: .local f64
+ store double %arg, %f64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load double, %f64_cell %retval
+ ; CHECK-NEXT: local.get 1
+ ; CHECK-NEXT: end_function
+ ret double %reloaded
+}
+
+define void @ir_unreferenced_local() {
+ ; CHECK-LABEL: ir_unreferenced_local:
+ ; CHECK-NEXT: .functype ir_unreferenced_local () -> ()
+ %unused = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ ret void
+ ; CHECK-NEXT: end_function
+}
Index: llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
@@ -0,0 +1,22 @@
+; RUN: llc -mtriple=wasm32-unknown-unknown -asm-verbose=false < %s | FileCheck %s --check-prefix=CHECKCG
+; RUN: llc -mtriple=wasm32-unknown-unknown -stop-after=finalize-isel < %s | FileCheck %s --check-prefix=CHECKISEL
+
+%f32_cell = type float addrspace(1)*
+
+; CHECKISEL-LABEL: name: ir_local_f32
+; CHECKISEL:   stack:
+; CHECKISEL:   id: 0, name: retval, type: default, offset: 1, size: 1, alignment: 4,
+; CHECKISEL-NEXT:  s

[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

[Andy Wingo via Phabricator via cfe-commits]

wingo reopened this revision.
wingo added a comment.
This revision is now accepted and ready to land.

Yarrrgh, broke the shared library build.  Fix incoming...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101140/new/

https://reviews.llvm.org/D101140

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

[Andy Wingo via Phabricator via cfe-commits]

wingo updated this revision to Diff 348758.
wingo added a comment.

Fix shared-library build by moving util function to WebAssemblyFrameLowering.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101140/new/

https://reviews.llvm.org/D101140

Files:
  llvm/include/llvm/CodeGen/MIRYamlMapping.h
  llvm/include/llvm/CodeGen/TargetFrameLowering.h
  llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
  llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyISD.def
  llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
  llvm/test/CodeGen/WebAssembly/ir-locals.ll

Index: llvm/test/CodeGen/WebAssembly/ir-locals.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals.ll
@@ -0,0 +1,87 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false | FileCheck %s
+
+%i32_cell = type i32 addrspace(1)*
+%i64_cell = type i64 addrspace(1)*
+%f32_cell = type float addrspace(1)*
+%f64_cell = type double addrspace(1)*
+
+; We have a set of tests in which we set a local and then reload the
+; local.  If the load immediately follows the set, the DAG combiner will
+; infer that the reloaded value is the same value that was set, which
+; isn't what we want to test.  To inhibit this optimization, we include
+; an opaque call between the store and the load.
+declare void @inhibit_store_to_load_forwarding()
+
+define i32 @ir_local_i32(i32 %arg) {
+ ; CHECK-LABEL: ir_local_i32:
+ ; CHECK-NEXT: .functype ir_local_i32 (i32) -> (i32)
+ %retval = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ store i32 %arg, %i32_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load i32, %i32_cell %retval
+ ; CHECK-NEXT: local.get 1
+ ret i32 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define i64 @ir_local_i64(i64 %arg) {
+ ; CHECK-LABEL: ir_local_i64:
+ ; CHECK-NEXT: .functype ir_local_i64 (i64) -> (i64)
+ %retval = alloca i64, addrspace(1)
+ ; CHECK-NEXT: .local i64
+ store i64 %arg, %i64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load i64, %i64_cell %retval
+ ; See note in ir_local_i32.
+ ; CHECK-NEXT: local.get 1
+ ret i64 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define float @ir_local_f32(float %arg) {
+ ; CHECK-LABEL: ir_local_f32:
+ ; CHECK-NEXT: .functype ir_local_f32 (f32) -> (f32)
+ %retval = alloca float, addrspace(1)
+ ; CHECK-NEXT: .local f32
+ store float %arg, %f32_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load float, %f32_cell %retval
+ ; CHECK-NEXT: local.get 1
+ ; CHECK-NEXT: end_function
+ ret float %reloaded
+}
+
+define double @ir_local_f64(double %arg) {
+ ; CHECK-LABEL: ir_local_f64:
+ ; CHECK-NEXT: .functype ir_local_f64 (f64) -> (f64)
+ %retval = alloca double, addrspace(1)
+ ; CHECK-NEXT: .local f64
+ store double %arg, %f64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load double, %f64_cell %retval
+ ; CHECK-NEXT: local.get 1
+ ; CHECK-NEXT: end_function
+ ret double %reloaded
+}
+
+define void @ir_unreferenced_local() {
+ ; CHECK-LABEL: ir_unreferenced_local:
+ ; CHECK-NEXT: .functype ir_unreferenced_local () -> ()
+ %unused = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ ret void
+ ; CHECK-NEXT: end_function
+}
Index: llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
@@ -0,0 +1,22 @@
+; RUN: llc -mtriple=wasm32-unknown-unknown -asm-verbose=false < %s | FileCheck %s --check-prefix=CHECKCG
+; RUN: llc -mtriple=wasm32-unknown-unknown -stop-after=finalize-isel < %s | FileCheck %s --check-prefix=CHECKISEL
+
+%f32_cell = type float addrspace(1)*
+
+; CHECKISEL-LABEL: name: ir_local_f32
+; CHECKISEL:   stack:
+; CHECKISEL:   id: 0, name: retval, type: default, offset: 1, size: 1, alignment: 4,
+; CHECKISEL-NEXT:  stack-id: wasm-local
+
+; CHECKCG-LABEL: ir_local_f32:
+; CHECKCG-NEXT: .functype ir_local_f32 (f32) -> (f32)
+; CHECKCG-NEXT: .local f32
+; CHECKCG-NEXT: local.get 0
+; CHECKCG-NEXT: local.set 1
+
+define float @ir_local_f32

[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

[Andy Wingo via Phabricator via cfe-commits]

wingo added a comment.

Quick r? to @tlively -- OK with having the new util function in 
WebAssemblyFrameLowering?  It didn't work to have it in the utils library 
because the function needs the vtable of WebAssemblyFunctionInfo.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101140/new/

https://reviews.llvm.org/D101140

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

[Andy Wingo via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG82f92e35c646: [WebAssembly][CodeGen] IR support for 
WebAssembly local variables (authored by wingo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101140/new/

https://reviews.llvm.org/D101140

Files:
  llvm/include/llvm/CodeGen/MIRYamlMapping.h
  llvm/include/llvm/CodeGen/TargetFrameLowering.h
  llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
  llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyISD.def
  llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
  llvm/test/CodeGen/WebAssembly/ir-locals.ll

Index: llvm/test/CodeGen/WebAssembly/ir-locals.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals.ll
@@ -0,0 +1,87 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false | FileCheck %s
+
+%i32_cell = type i32 addrspace(1)*
+%i64_cell = type i64 addrspace(1)*
+%f32_cell = type float addrspace(1)*
+%f64_cell = type double addrspace(1)*
+
+; We have a set of tests in which we set a local and then reload the
+; local.  If the load immediately follows the set, the DAG combiner will
+; infer that the reloaded value is the same value that was set, which
+; isn't what we want to test.  To inhibit this optimization, we include
+; an opaque call between the store and the load.
+declare void @inhibit_store_to_load_forwarding()
+
+define i32 @ir_local_i32(i32 %arg) {
+ ; CHECK-LABEL: ir_local_i32:
+ ; CHECK-NEXT: .functype ir_local_i32 (i32) -> (i32)
+ %retval = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ store i32 %arg, %i32_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load i32, %i32_cell %retval
+ ; CHECK-NEXT: local.get 1
+ ret i32 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define i64 @ir_local_i64(i64 %arg) {
+ ; CHECK-LABEL: ir_local_i64:
+ ; CHECK-NEXT: .functype ir_local_i64 (i64) -> (i64)
+ %retval = alloca i64, addrspace(1)
+ ; CHECK-NEXT: .local i64
+ store i64 %arg, %i64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load i64, %i64_cell %retval
+ ; See note in ir_local_i32.
+ ; CHECK-NEXT: local.get 1
+ ret i64 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define float @ir_local_f32(float %arg) {
+ ; CHECK-LABEL: ir_local_f32:
+ ; CHECK-NEXT: .functype ir_local_f32 (f32) -> (f32)
+ %retval = alloca float, addrspace(1)
+ ; CHECK-NEXT: .local f32
+ store float %arg, %f32_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load float, %f32_cell %retval
+ ; CHECK-NEXT: local.get 1
+ ; CHECK-NEXT: end_function
+ ret float %reloaded
+}
+
+define double @ir_local_f64(double %arg) {
+ ; CHECK-LABEL: ir_local_f64:
+ ; CHECK-NEXT: .functype ir_local_f64 (f64) -> (f64)
+ %retval = alloca double, addrspace(1)
+ ; CHECK-NEXT: .local f64
+ store double %arg, %f64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ call void @inhibit_store_to_load_forwarding()
+ ; CHECK-NEXT: call inhibit_store_to_load_forwarding
+ %reloaded = load double, %f64_cell %retval
+ ; CHECK-NEXT: local.get 1
+ ; CHECK-NEXT: end_function
+ ret double %reloaded
+}
+
+define void @ir_unreferenced_local() {
+ ; CHECK-LABEL: ir_unreferenced_local:
+ ; CHECK-NEXT: .functype ir_unreferenced_local () -> ()
+ %unused = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ ret void
+ ; CHECK-NEXT: end_function
+}
Index: llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
@@ -0,0 +1,22 @@
+; RUN: llc -mtriple=wasm32-unknown-unknown -asm-verbose=false < %s | FileCheck %s --check-prefix=CHECKCG
+; RUN: llc -mtriple=wasm32-unknown-unknown -stop-after=finalize-isel < %s | FileCheck %s --check-prefix=CHECKISEL
+
+%f32_cell = type float addrspace(1)*
+
+; CHECKISEL-LABEL: name: ir_local_f32
+; CHECKISEL:   stack:
+; CHECKISEL:   id: 0, name: retval, type: default, offset: 1, size: 1, alignment: 4,
+; CHECKISEL-NEXT:  stack-id: wasm-local
+
+; CHECKCG-LABEL: ir_local_f32:
+; CHECKCG-NEXT: .functype ir_local_f32 (f32) -> (f32)
+; CHECKCG-

[PATCH] D108360: [clang][NFC] Remove dead code

[Andy Wingo via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGeb3af1e77341: [clang][NFC] Remove dead code (authored by 
wingo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108360/new/

https://reviews.llvm.org/D108360

Files:
  clang/lib/Sema/SemaType.cpp


Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -8310,10 +8310,6 @@
   attr.getMacroExpansionLoc());
 }
   }
-
-  if (!state.getSema().getLangOpts().OpenCL ||
-  type.getAddressSpace() != LangAS::Default)
-return;
 }
 
 void Sema::completeExprArrayBound(Expr *E) {


Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -8310,10 +8310,6 @@
   attr.getMacroExpansionLoc());
 }
   }
-
-  if (!state.getSema().getLangOpts().OpenCL ||
-  type.getAddressSpace() != LangAS::Default)
-return;
 }
 
 void Sema::completeExprArrayBound(Expr *E) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108359: [clang][NFC] Fix needless double-parenthisation

[Andy Wingo via Phabricator via cfe-commits]

wingo added a reviewer: Anastasia.
wingo added a comment.

Just a little thing I saw while reading code -- if no response in a couple 
weeks I will just abandon the rev


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108359/new/

https://reviews.llvm.org/D108359

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108359: [clang][NFC] Fix needless double-parenthisation

[Andy Wingo via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9ae4275557ca: [clang][NFC] Fix needless 
double-parenthisation (authored by wingo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108359/new/

https://reviews.llvm.org/D108359

Files:
  clang/lib/Sema/TreeTransform.h


Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -4745,8 +4745,8 @@
   SourceLocation Loc = TL.getBeginLoc();
   Qualifiers Quals = TL.getType().getLocalQualifiers();
 
-  if (((T.getAddressSpace() != LangAS::Default &&
-Quals.getAddressSpace() != LangAS::Default)) &&
+  if ((T.getAddressSpace() != LangAS::Default &&
+   Quals.getAddressSpace() != LangAS::Default) &&
   T.getAddressSpace() != Quals.getAddressSpace()) {
 SemaRef.Diag(Loc, diag::err_address_space_mismatch_templ_inst)
 << TL.getType() << T;


Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -4745,8 +4745,8 @@
   SourceLocation Loc = TL.getBeginLoc();
   Qualifiers Quals = TL.getType().getLocalQualifiers();
 
-  if (((T.getAddressSpace() != LangAS::Default &&
-Quals.getAddressSpace() != LangAS::Default)) &&
+  if ((T.getAddressSpace() != LangAS::Default &&
+   Quals.getAddressSpace() != LangAS::Default) &&
   T.getAddressSpace() != Quals.getAddressSpace()) {
 SemaRef.Diag(Loc, diag::err_address_space_mismatch_templ_inst)
 << TL.getType() << T;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108359: [clang][NFC] Fix needless double-parenthisation

[Andy Wingo via Phabricator via cfe-commits]

wingo created this revision.
wingo requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Strip a layer of parentheses in TreeTransform::RebuildQualifiedType.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108359

Files:
  clang/lib/Sema/TreeTransform.h


Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -4745,8 +4745,8 @@
   SourceLocation Loc = TL.getBeginLoc();
   Qualifiers Quals = TL.getType().getLocalQualifiers();
 
-  if (((T.getAddressSpace() != LangAS::Default &&
-Quals.getAddressSpace() != LangAS::Default)) &&
+  if ((T.getAddressSpace() != LangAS::Default &&
+   Quals.getAddressSpace() != LangAS::Default) &&
   T.getAddressSpace() != Quals.getAddressSpace()) {
 SemaRef.Diag(Loc, diag::err_address_space_mismatch_templ_inst)
 << TL.getType() << T;


Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -4745,8 +4745,8 @@
   SourceLocation Loc = TL.getBeginLoc();
   Qualifiers Quals = TL.getType().getLocalQualifiers();
 
-  if (((T.getAddressSpace() != LangAS::Default &&
-Quals.getAddressSpace() != LangAS::Default)) &&
+  if ((T.getAddressSpace() != LangAS::Default &&
+   Quals.getAddressSpace() != LangAS::Default) &&
   T.getAddressSpace() != Quals.getAddressSpace()) {
 SemaRef.Diag(Loc, diag::err_address_space_mismatch_templ_inst)
 << TL.getType() << T;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108360: [clang][NFC] Remove dead code

[Andy Wingo via Phabricator via cfe-commits]

wingo created this revision.
wingo requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Remove code that has no effect in SemaType.cpp:processTypeAttrs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108360

Files:
  clang/lib/Sema/SemaType.cpp


Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -8303,10 +8303,6 @@
   attr.getMacroExpansionLoc());
 }
   }
-
-  if (!state.getSema().getLangOpts().OpenCL ||
-  type.getAddressSpace() != LangAS::Default)
-return;
 }
 
 void Sema::completeExprArrayBound(Expr *E) {


Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -8303,10 +8303,6 @@
   attr.getMacroExpansionLoc());
 }
   }
-
-  if (!state.getSema().getLangOpts().OpenCL ||
-  type.getAddressSpace() != LangAS::Default)
-return;
 }
 
 void Sema::completeExprArrayBound(Expr *E) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108359: [clang][NFC] Fix needless double-parenthisation

[Andy Wingo via Phabricator via cfe-commits]

wingo added a reviewer: rsmith.
wingo added a comment.

Hi Richard, I am new to clang, having mostly worked on the WebAssembly target 
in llvm proper -- I have a stack of patches, of which a couple later ones touch 
Sema/.  See https://lists.llvm.org/pipermail/cfe-dev/2021-July/068559.html for 
broader context.  However I had a couple small NFC cleanups I found when 
reading code, like this one; would it be OK if I sent them your way?  Cheers, 
Andy.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108359/new/

https://reviews.llvm.org/D108359

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108360: [clang][NFC] Remove dead code

[Andy Wingo via Phabricator via cfe-commits]

wingo added a reviewer: rjmccall.
wingo added a comment.

Hi John, I am new to clang, having mostly worked on the WebAssembly target in 
llvm proper.  I have a stack of patches related to address space treatment in 
codegen -- most of them NFC refactors to later allow the WebAssembly target to 
alloca in different address spaces. See 
https://lists.llvm.org/pipermail/cfe-dev/2021-July/068559.html for broader 
context. Would it be OK if I sent them your way? Cheers, Andy.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108360/new/

https://reviews.llvm.org/D108360

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR

[Andy Wingo via Phabricator via cfe-commits]

wingo updated this revision to Diff 341868.
wingo added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fix clang datalayout expectations


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101608/new/

https://reviews.llvm.org/D101608

Files:
  clang/lib/Basic/Targets/WebAssembly.h
  clang/test/CodeGen/target-data.c
  llvm/lib/Target/WebAssembly/WebAssemblyISD.def
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/test/CodeGen/WebAssembly/global-get.ll
  llvm/test/CodeGen/WebAssembly/global-set.ll

Index: llvm/test/CodeGen/WebAssembly/global-set.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/global-set.ll
@@ -0,0 +1,57 @@
+; RUN: llc --mtriple=wasm32-unknown-unknown -asm-verbose=false < %s | FileCheck %s
+
+@i32_global = local_unnamed_addr addrspace(1) global i32 undef
+@i64_global = local_unnamed_addr addrspace(1) global i64 undef
+@f32_global = local_unnamed_addr addrspace(1) global float undef
+@f64_global = local_unnamed_addr addrspace(1) global double undef
+
+define void @set_i32_global(i32 %v) {
+; CHECK-LABEL: set_i32_global:
+; CHECK-NEXT: functype   set_i32_global (i32) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set i32_global
+; CHECK-NEXT: end_function
+  store i32 %v, i32 addrspace(1)* @i32_global
+  ret void
+}
+
+define void @set_i64_global(i64 %v) {
+; CHECK-LABEL: set_i64_global:
+; CHECK-NEXT: functype   set_i64_global (i64) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set i64_global
+; CHECK-NEXT: end_function
+  store i64 %v, i64 addrspace(1)* @i64_global
+  ret void
+}
+
+define void @set_f32_global(float %v) {
+; CHECK-LABEL: set_f32_global:
+; CHECK-NEXT: functype   set_f32_global (f32) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set f32_global
+; CHECK-NEXT: end_function
+  store float %v, float addrspace(1)* @f32_global
+  ret void
+}
+
+define void @set_f64_global(double %v) {
+; CHECK-LABEL: set_f64_global:
+; CHECK-NEXT: functype   set_f64_global (f64) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set f64_global
+; CHECK-NEXT: end_function
+  store double %v, double addrspace(1)* @f64_global
+  ret void
+}
+
+;; LLVM doesn't yet declare proper WebAssembly globals for these values,
+;; instead placing them in linear memory.  To fix in a followup.
+; FIXME-CHECK: .globl i32_global
+; FIXME-CHECK: .globaltype i32_global, i32
+; FIXME-CHECK: .globl i64_global
+; FIXME-CHECK: .globaltype i64_global, i64
+; FIXME-CHECK: .globl f32_global
+; FIXME-CHECK: .globaltype f32_global, f32
+; FIXME-CHECK: .globl f64_global
+; FIXME-CHECK: .globaltype f64_global, f64
Index: llvm/test/CodeGen/WebAssembly/global-get.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/global-get.ll
@@ -0,0 +1,54 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false | FileCheck %s
+
+@i32_global = local_unnamed_addr addrspace(1) global i32 undef
+@i64_global = local_unnamed_addr addrspace(1) global i64 undef
+@f32_global = local_unnamed_addr addrspace(1) global float undef
+@f64_global = local_unnamed_addr addrspace(1) global double undef
+
+define i32 @return_i32_global() {
+; CHECK-LABEL: return_i32_global:
+; CHECK-NEXT: functype   return_i32_global () -> (i32)
+; CHECK-NEXT: global.get i32_global
+; CHECK-NEXT: end_function
+  %v = load i32, i32 addrspace(1)* @i32_global
+  ret i32 %v
+}
+
+define i64 @return_i64_global() {
+; CHECK-LABEL: return_i64_global:
+; CHECK-NEXT: functype   return_i64_global () -> (i64)
+; CHECK-NEXT: global.get i64_global
+; CHECK-NEXT: end_function
+  %v = load i64, i64 addrspace(1)* @i64_global
+  ret i64 %v
+}
+
+define float @return_f32_global() {
+; CHECK-LABEL: return_f32_global:
+; CHECK-NEXT: functype   return_f32_global () -> (f32)
+; CHECK-NEXT: global.get f32_global
+; CHECK-NEXT: end_function
+  %v = load float, float addrspace(1)* @f32_global
+  ret float %v
+}
+
+define double @return_f64_global() {
+; CHECK-LABEL: return_f64_global:
+; CHECK-NEXT: functype   return_f64_global () -> (f64)
+; CHECK-NEXT: global.get f64_global
+; CHECK-NEXT: end_function
+  %v = load double, double addrspace(1)* @f64_global
+  ret double %v
+}
+
+
+;; LLVM doesn't yet declare proper WebAssembly globals for these values,
+;; instead placing them in linear memory.  To fix in a followup.
+; FIXME-CHECK: .globl i32_global
+; FIXME-CHECK: .globaltype i32_global, i32
+; FIXME-CHECK: .globl i64_global
+; FIXME-CHECK: .globaltype i64_global, i64
+; FIXME-CHECK: .globl f32_global
+; FIXME-CHECK: .globaltype f32_global, f32
+; FIXME-CHECK: .globl f64_global
+; FIXME-CHECK: .globaltype f64_global, f64
Index: llvm/li

[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

[Andy Wingo via Phabricator via cfe-commits]

wingo updated this revision to Diff 342698.
wingo added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fix clang datalayout expectations


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101140/new/

https://reviews.llvm.org/D101140

Files:
  clang/lib/Basic/Targets/WebAssembly.h
  clang/test/CodeGen/target-data.c
  llvm/include/llvm/CodeGen/MIRYamlMapping.h
  llvm/include/llvm/CodeGen/TargetFrameLowering.h
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h
  llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyISD.def
  llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/test/CodeGen/WebAssembly/ir-locals.ll

Index: llvm/test/CodeGen/WebAssembly/ir-locals.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals.ll
@@ -0,0 +1,76 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false | FileCheck %s
+
+%i32_cell = type i32 addrspace(1)*
+%i64_cell = type i64 addrspace(1)*
+%f32_cell = type float addrspace(1)*
+%f64_cell = type double addrspace(1)*
+
+define i32 @ir_local_i32(i32 %arg) {
+ ; CHECK-LABEL: ir_local_i32:
+ ; CHECK-NEXT: .functype ir_local_i32 (i32) -> (i32)
+ %retval = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ store i32 %arg, %i32_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ %reloaded = load i32, %i32_cell %retval
+ ; The DAG combiner infers that %reloaded is the same as %arg and
+ ; ultimately causes "local.get 0" to be emitted instead of
+ ; "local.get 1".
+ ; CHECK-NEXT: local.get 0
+ ret i32 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define i64 @ir_local_i64(i64 %arg) {
+ ; CHECK-LABEL: ir_local_i64:
+ ; CHECK-NEXT: .functype ir_local_i64 (i64) -> (i64)
+ %retval = alloca i64, addrspace(1)
+ ; CHECK-NEXT: .local i64
+ store i64 %arg, %i64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ %reloaded = load i64, %i64_cell %retval
+ ; See note in ir_local_i32.
+ ; CHECK-NEXT: local.get 0
+ ret i64 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define float @ir_local_f32(float %arg) {
+ ; CHECK-LABEL: ir_local_f32:
+ ; CHECK-NEXT: .functype ir_local_f32 (f32) -> (f32)
+ %retval = alloca float, addrspace(1)
+ ; CHECK-NEXT: .local f32
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ store float %arg, %f32_cell %retval
+ ; See note in ir_local_i32.
+ ; CHECK-NEXT: local.get 0
+ %reloaded = load float, %f32_cell %retval
+ ; CHECK-NEXT: end_function
+ ret float %reloaded
+}
+
+define double @ir_local_f64(double %arg) {
+ ; CHECK-LABEL: ir_local_f64:
+ ; CHECK-NEXT: .functype ir_local_f64 (f64) -> (f64)
+ %retval = alloca double, addrspace(1)
+ ; CHECK-NEXT: .local f64
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ store double %arg, %f64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ %reloaded = load double, %f64_cell %retval
+ ; CHECK-NEXT: end_function
+ ret double %reloaded
+}
+
+define void @ir_unreferenced_local() {
+ ; CHECK-LABEL: ir_unreferenced_local:
+ ; CHECK-NEXT: .functype ir_unreferenced_local () -> ()
+ %unused = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ ret void
+ ; CHECK-NEXT: end_function
+}
Index: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -120,8 +120,8 @@
 Optional CM, CodeGenOpt::Level OL, bool JIT)
 : LLVMTargetMachine(T,
 TT.isArch64Bit()
-? "e-m:e-p:64:64-i64:64-n32:64-S128-ni:1"
-: "e-m:e-p:32:32-i64:64-n32:64-S128-ni:1",
+? "e-m:e-p:64:64-i64:64-n32:64-S128-ni:1-A0:u"
+: "e-m:e-p:32:32-i64:64-n32:64-S128-ni:1-A0:u",
 TT, CPU, FS, Options, getEffectiveRelocModel(RM, TT),
 getEffectiveCodeModel(CM, CodeModel::Large), OL),
   TLOF(new WebAssemblyTargetObjectFile()) {
Index: llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
===
--- llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
+++ llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
@@ -72,6 +72,8 @@
 SDCallSeqEnd<[SDTCisVT<0, iPTR>, SDTCisVT<1, iPTR>]>;
 def SDT_WebAssemblyBrTable: SDTypePro

[PATCH] D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR

[Andy Wingo via Phabricator via cfe-commits]

wingo updated this revision to Diff 342719.
wingo added a comment.

Rebase on main


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101608/new/

https://reviews.llvm.org/D101608

Files:
  clang/lib/Basic/Targets/WebAssembly.h
  clang/test/CodeGen/target-data.c
  llvm/lib/Target/WebAssembly/WebAssemblyISD.def
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/test/CodeGen/WebAssembly/global-get.ll
  llvm/test/CodeGen/WebAssembly/global-set.ll

Index: llvm/test/CodeGen/WebAssembly/global-set.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/global-set.ll
@@ -0,0 +1,57 @@
+; RUN: llc --mtriple=wasm32-unknown-unknown -asm-verbose=false < %s | FileCheck %s
+
+@i32_global = local_unnamed_addr addrspace(1) global i32 undef
+@i64_global = local_unnamed_addr addrspace(1) global i64 undef
+@f32_global = local_unnamed_addr addrspace(1) global float undef
+@f64_global = local_unnamed_addr addrspace(1) global double undef
+
+define void @set_i32_global(i32 %v) {
+; CHECK-LABEL: set_i32_global:
+; CHECK-NEXT: functype   set_i32_global (i32) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set i32_global
+; CHECK-NEXT: end_function
+  store i32 %v, i32 addrspace(1)* @i32_global
+  ret void
+}
+
+define void @set_i64_global(i64 %v) {
+; CHECK-LABEL: set_i64_global:
+; CHECK-NEXT: functype   set_i64_global (i64) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set i64_global
+; CHECK-NEXT: end_function
+  store i64 %v, i64 addrspace(1)* @i64_global
+  ret void
+}
+
+define void @set_f32_global(float %v) {
+; CHECK-LABEL: set_f32_global:
+; CHECK-NEXT: functype   set_f32_global (f32) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set f32_global
+; CHECK-NEXT: end_function
+  store float %v, float addrspace(1)* @f32_global
+  ret void
+}
+
+define void @set_f64_global(double %v) {
+; CHECK-LABEL: set_f64_global:
+; CHECK-NEXT: functype   set_f64_global (f64) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set f64_global
+; CHECK-NEXT: end_function
+  store double %v, double addrspace(1)* @f64_global
+  ret void
+}
+
+;; LLVM doesn't yet declare proper WebAssembly globals for these values,
+;; instead placing them in linear memory.  To fix in a followup.
+; FIXME-CHECK: .globl i32_global
+; FIXME-CHECK: .globaltype i32_global, i32
+; FIXME-CHECK: .globl i64_global
+; FIXME-CHECK: .globaltype i64_global, i64
+; FIXME-CHECK: .globl f32_global
+; FIXME-CHECK: .globaltype f32_global, f32
+; FIXME-CHECK: .globl f64_global
+; FIXME-CHECK: .globaltype f64_global, f64
Index: llvm/test/CodeGen/WebAssembly/global-get.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/global-get.ll
@@ -0,0 +1,54 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false | FileCheck %s
+
+@i32_global = local_unnamed_addr addrspace(1) global i32 undef
+@i64_global = local_unnamed_addr addrspace(1) global i64 undef
+@f32_global = local_unnamed_addr addrspace(1) global float undef
+@f64_global = local_unnamed_addr addrspace(1) global double undef
+
+define i32 @return_i32_global() {
+; CHECK-LABEL: return_i32_global:
+; CHECK-NEXT: functype   return_i32_global () -> (i32)
+; CHECK-NEXT: global.get i32_global
+; CHECK-NEXT: end_function
+  %v = load i32, i32 addrspace(1)* @i32_global
+  ret i32 %v
+}
+
+define i64 @return_i64_global() {
+; CHECK-LABEL: return_i64_global:
+; CHECK-NEXT: functype   return_i64_global () -> (i64)
+; CHECK-NEXT: global.get i64_global
+; CHECK-NEXT: end_function
+  %v = load i64, i64 addrspace(1)* @i64_global
+  ret i64 %v
+}
+
+define float @return_f32_global() {
+; CHECK-LABEL: return_f32_global:
+; CHECK-NEXT: functype   return_f32_global () -> (f32)
+; CHECK-NEXT: global.get f32_global
+; CHECK-NEXT: end_function
+  %v = load float, float addrspace(1)* @f32_global
+  ret float %v
+}
+
+define double @return_f64_global() {
+; CHECK-LABEL: return_f64_global:
+; CHECK-NEXT: functype   return_f64_global () -> (f64)
+; CHECK-NEXT: global.get f64_global
+; CHECK-NEXT: end_function
+  %v = load double, double addrspace(1)* @f64_global
+  ret double %v
+}
+
+
+;; LLVM doesn't yet declare proper WebAssembly globals for these values,
+;; instead placing them in linear memory.  To fix in a followup.
+; FIXME-CHECK: .globl i32_global
+; FIXME-CHECK: .globaltype i32_global, i32
+; FIXME-CHECK: .globl i64_global
+; FIXME-CHECK: .globaltype i64_global, i64
+; FIXME-CHECK: .globl f32_global
+; FIXME-CHECK: .globaltype f32_global, f32
+; FIXME-CHECK: .globl f64_global
+; FIXME-CHECK: .globaltype f64_global, f64
Index: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

[PATCH] D95425: Implementation of global.get/set for reftypes in LLVM IR

[Andy Wingo via Phabricator via cfe-commits]

wingo added inline comments.



Comment at: clang/lib/Basic/Targets/WebAssembly.h:150
   : WebAssemblyTargetInfo(T, Opts) {
-resetDataLayout("e-m:e-p:32:32-i64:64-n32:64-S128");
+resetDataLayout("e-m:e-p:32:32-i64:64-n32:64-S128-ni:1");
   }

This change is already in D101608; rebase?



Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:4083
   EVT PtrVT = Ptr.getValueType();
+  EVT EltVT = PtrVT.getScalarType();
 

It turns out that all the changes here to `visitLoad` and `visitStore` are no 
longer needed.  The issue was that before we made `getPointerMemTy` virtual, it 
was hard-coded to return an integer of whatever the pointer bit size was.  But 
for externref and funcref values, where we're using pointers in non-default 
address spaces to represent opaque values, that's not what we want: an 
externref "in memory" isn't an i32.  Having made `getPointerMemTy` virtual and 
returning externref / funcref for those values, now we avoid the zero-extension 
paths, and even the ISD::ADD node doesn't cause us problems.



Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:4257
+// Skip ZExt or Trunc if a ValueVT is zero sized
+if (!ValueVTs[i].isZeroSized() && MemVTs[i] != ValueVTs[i])
   Val = DAG.getPtrExtOrTrunc(Val, dl, MemVTs[i]);

Can revert this too, as mentioned above.



Comment at: llvm/lib/CodeGen/TargetLoweringBase.cpp:1684
   Type *Ty = VT.getTypeForEVT(Context);
-  if (Alignment >= DL.getABITypeAlign(Ty)) {
+  if (!VT.isZeroSized() && Alignment >= DL.getABITypeAlign(Ty)) {
 // Assume that an access that meets the ABI-specified alignment is fast.

This one I would think that we want to return true, that the access is "fast", 
for access to zero-sized types.



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:2347
+  return false;
+}

I just checked and it would seem that the `getPointerMemTy` change means that 
this override no longer appears to be needed.



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyInstrTable.td:15
 multiclass TABLE {
+  let mayLoad = 1 in
   defm TABLE_GET_#rt : I<(outs rt:$res), (ins table32_op:$table),

I think you may need `hasSideEffects = 0` for these annotations to have an 
effect.



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyInstrTable.td:19
  [],
  "table.get\t$res, $table",
  "table.get\t$table",

Not something for this patch, but this is certainly bogus: surely we mean 
`table.get\t$table, $i` and we have an i32 index argument?



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyInstrTable.td:59
+  (TABLE_SET_EXTERNREF i32:$table, i32:$idx, externref:$r)>,
+  Requires<[HasReferenceTypes]>;
+

Consider changing to use the approach used for `GLOBAL_GET` from the parent 
commit, and folding these into the multiclass so that we don't have to repeat 
the code for externref and funcref.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95425/new/

https://reviews.llvm.org/D95425

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR

[Andy Wingo via Phabricator via cfe-commits]

wingo marked 3 inline comments as done.
wingo added inline comments.



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1324
+{LN->getChain(), Base}, LN->getMemoryVT(), LN->getMemOperand());
+return DAG.getMergeValues({GlobalGet, LN->getChain()}, DL);
+  }

tlively wrote:
> I'm not sure it's necessary to create a MERGE_NODES node here, since the the 
> GlobalGet already contains the chain. For a similar example, see how 
> LOAD_SPLAT nodes are created and returned.
I think it is 



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1324
+{LN->getChain(), Base}, LN->getMemoryVT(), LN->getMemOperand());
+return DAG.getMergeValues({GlobalGet, LN->getChain()}, DL);
+  }

wingo wrote:
> tlively wrote:
> > I'm not sure it's necessary to create a MERGE_NODES node here, since the 
> > the GlobalGet already contains the chain. For a similar example, see how 
> > LOAD_SPLAT nodes are created and returned.
> I think it is 
Aaaah thanks for this tip!  Took some finagling to find the right formula but I 
got it now.



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td:268
 // are implied by virtual register uses and defs.
-multiclass LOCAL {
+multiclass LOCAL {
   let hasSideEffects = 0 in {

tlively wrote:
> In other places we have used the abbreviation `rc` as the name for 
> WebAssemblyRegClass parameters, but I don't know if we do that everywhere.
Good tip; apparently it's consistent except in WebAssemblyInstrRef.td, where I 
had initiailly looked.  Will go ahead and fix that one too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101608/new/

https://reviews.llvm.org/D101608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR

[Andy Wingo via Phabricator via cfe-commits]

wingo updated this revision to Diff 342963.
wingo marked 2 inline comments as done.
wingo added a comment.

Address feedback


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101608/new/

https://reviews.llvm.org/D101608

Files:
  clang/lib/Basic/Targets/WebAssembly.h
  clang/test/CodeGen/target-data.c
  llvm/lib/Target/WebAssembly/WebAssemblyISD.def
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrRef.td
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/test/CodeGen/WebAssembly/global-get.ll
  llvm/test/CodeGen/WebAssembly/global-set.ll

Index: llvm/test/CodeGen/WebAssembly/global-set.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/global-set.ll
@@ -0,0 +1,57 @@
+; RUN: llc --mtriple=wasm32-unknown-unknown -asm-verbose=false < %s | FileCheck %s
+
+@i32_global = local_unnamed_addr addrspace(1) global i32 undef
+@i64_global = local_unnamed_addr addrspace(1) global i64 undef
+@f32_global = local_unnamed_addr addrspace(1) global float undef
+@f64_global = local_unnamed_addr addrspace(1) global double undef
+
+define void @set_i32_global(i32 %v) {
+; CHECK-LABEL: set_i32_global:
+; CHECK-NEXT: functype   set_i32_global (i32) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set i32_global
+; CHECK-NEXT: end_function
+  store i32 %v, i32 addrspace(1)* @i32_global
+  ret void
+}
+
+define void @set_i64_global(i64 %v) {
+; CHECK-LABEL: set_i64_global:
+; CHECK-NEXT: functype   set_i64_global (i64) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set i64_global
+; CHECK-NEXT: end_function
+  store i64 %v, i64 addrspace(1)* @i64_global
+  ret void
+}
+
+define void @set_f32_global(float %v) {
+; CHECK-LABEL: set_f32_global:
+; CHECK-NEXT: functype   set_f32_global (f32) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set f32_global
+; CHECK-NEXT: end_function
+  store float %v, float addrspace(1)* @f32_global
+  ret void
+}
+
+define void @set_f64_global(double %v) {
+; CHECK-LABEL: set_f64_global:
+; CHECK-NEXT: functype   set_f64_global (f64) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set f64_global
+; CHECK-NEXT: end_function
+  store double %v, double addrspace(1)* @f64_global
+  ret void
+}
+
+;; LLVM doesn't yet declare proper WebAssembly globals for these values,
+;; instead placing them in linear memory.  To fix in a followup.
+; FIXME-CHECK: .globl i32_global
+; FIXME-CHECK: .globaltype i32_global, i32
+; FIXME-CHECK: .globl i64_global
+; FIXME-CHECK: .globaltype i64_global, i64
+; FIXME-CHECK: .globl f32_global
+; FIXME-CHECK: .globaltype f32_global, f32
+; FIXME-CHECK: .globl f64_global
+; FIXME-CHECK: .globaltype f64_global, f64
Index: llvm/test/CodeGen/WebAssembly/global-get.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/global-get.ll
@@ -0,0 +1,54 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false | FileCheck %s
+
+@i32_global = local_unnamed_addr addrspace(1) global i32 undef
+@i64_global = local_unnamed_addr addrspace(1) global i64 undef
+@f32_global = local_unnamed_addr addrspace(1) global float undef
+@f64_global = local_unnamed_addr addrspace(1) global double undef
+
+define i32 @return_i32_global() {
+; CHECK-LABEL: return_i32_global:
+; CHECK-NEXT: functype   return_i32_global () -> (i32)
+; CHECK-NEXT: global.get i32_global
+; CHECK-NEXT: end_function
+  %v = load i32, i32 addrspace(1)* @i32_global
+  ret i32 %v
+}
+
+define i64 @return_i64_global() {
+; CHECK-LABEL: return_i64_global:
+; CHECK-NEXT: functype   return_i64_global () -> (i64)
+; CHECK-NEXT: global.get i64_global
+; CHECK-NEXT: end_function
+  %v = load i64, i64 addrspace(1)* @i64_global
+  ret i64 %v
+}
+
+define float @return_f32_global() {
+; CHECK-LABEL: return_f32_global:
+; CHECK-NEXT: functype   return_f32_global () -> (f32)
+; CHECK-NEXT: global.get f32_global
+; CHECK-NEXT: end_function
+  %v = load float, float addrspace(1)* @f32_global
+  ret float %v
+}
+
+define double @return_f64_global() {
+; CHECK-LABEL: return_f64_global:
+; CHECK-NEXT: functype   return_f64_global () -> (f64)
+; CHECK-NEXT: global.get f64_global
+; CHECK-NEXT: end_function
+  %v = load double, double addrspace(1)* @f64_global
+  ret double %v
+}
+
+
+;; LLVM doesn't yet declare proper WebAssembly globals for these values,
+;; instead placing them in linear memory.  To fix in a followup.
+; FIXME-CHECK: .globl i32_global
+; FIXME-CHECK: .globaltype i32_global, i32
+; FIXME-CHECK: .globl i64_global
+; FIXME-CHECK: .globaltype i64_global, i64
+; FIXME-CHECK: .globl f32_global
+; FIXME-CHECK: .globaltype f32_global, f32
+; FIXME-CHECK: .globl f64_global
+; FIXME-CHECK: .globaltype f64_global, f64
Index: ll

[PATCH] D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR

[Andy Wingo via Phabricator via cfe-commits]

wingo updated this revision to Diff 342970.
wingo added a comment.

Pull in fixes from D101140 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101608/new/

https://reviews.llvm.org/D101608

Files:
  clang/lib/Basic/Targets/WebAssembly.h
  clang/test/CodeGen/target-data.c
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h
  llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISD.def
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrRef.td
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/test/CodeGen/WebAssembly/global-get.ll
  llvm/test/CodeGen/WebAssembly/global-set.ll

Index: llvm/test/CodeGen/WebAssembly/global-set.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/global-set.ll
@@ -0,0 +1,57 @@
+; RUN: llc --mtriple=wasm32-unknown-unknown -asm-verbose=false < %s | FileCheck %s
+
+@i32_global = local_unnamed_addr addrspace(1) global i32 undef
+@i64_global = local_unnamed_addr addrspace(1) global i64 undef
+@f32_global = local_unnamed_addr addrspace(1) global float undef
+@f64_global = local_unnamed_addr addrspace(1) global double undef
+
+define void @set_i32_global(i32 %v) {
+; CHECK-LABEL: set_i32_global:
+; CHECK-NEXT: functype   set_i32_global (i32) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set i32_global
+; CHECK-NEXT: end_function
+  store i32 %v, i32 addrspace(1)* @i32_global
+  ret void
+}
+
+define void @set_i64_global(i64 %v) {
+; CHECK-LABEL: set_i64_global:
+; CHECK-NEXT: functype   set_i64_global (i64) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set i64_global
+; CHECK-NEXT: end_function
+  store i64 %v, i64 addrspace(1)* @i64_global
+  ret void
+}
+
+define void @set_f32_global(float %v) {
+; CHECK-LABEL: set_f32_global:
+; CHECK-NEXT: functype   set_f32_global (f32) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set f32_global
+; CHECK-NEXT: end_function
+  store float %v, float addrspace(1)* @f32_global
+  ret void
+}
+
+define void @set_f64_global(double %v) {
+; CHECK-LABEL: set_f64_global:
+; CHECK-NEXT: functype   set_f64_global (f64) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set f64_global
+; CHECK-NEXT: end_function
+  store double %v, double addrspace(1)* @f64_global
+  ret void
+}
+
+;; LLVM doesn't yet declare proper WebAssembly globals for these values,
+;; instead placing them in linear memory.  To fix in a followup.
+; FIXME-CHECK: .globl i32_global
+; FIXME-CHECK: .globaltype i32_global, i32
+; FIXME-CHECK: .globl i64_global
+; FIXME-CHECK: .globaltype i64_global, i64
+; FIXME-CHECK: .globl f32_global
+; FIXME-CHECK: .globaltype f32_global, f32
+; FIXME-CHECK: .globl f64_global
+; FIXME-CHECK: .globaltype f64_global, f64
Index: llvm/test/CodeGen/WebAssembly/global-get.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/global-get.ll
@@ -0,0 +1,54 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false | FileCheck %s
+
+@i32_global = local_unnamed_addr addrspace(1) global i32 undef
+@i64_global = local_unnamed_addr addrspace(1) global i64 undef
+@f32_global = local_unnamed_addr addrspace(1) global float undef
+@f64_global = local_unnamed_addr addrspace(1) global double undef
+
+define i32 @return_i32_global() {
+; CHECK-LABEL: return_i32_global:
+; CHECK-NEXT: functype   return_i32_global () -> (i32)
+; CHECK-NEXT: global.get i32_global
+; CHECK-NEXT: end_function
+  %v = load i32, i32 addrspace(1)* @i32_global
+  ret i32 %v
+}
+
+define i64 @return_i64_global() {
+; CHECK-LABEL: return_i64_global:
+; CHECK-NEXT: functype   return_i64_global () -> (i64)
+; CHECK-NEXT: global.get i64_global
+; CHECK-NEXT: end_function
+  %v = load i64, i64 addrspace(1)* @i64_global
+  ret i64 %v
+}
+
+define float @return_f32_global() {
+; CHECK-LABEL: return_f32_global:
+; CHECK-NEXT: functype   return_f32_global () -> (f32)
+; CHECK-NEXT: global.get f32_global
+; CHECK-NEXT: end_function
+  %v = load float, float addrspace(1)* @f32_global
+  ret float %v
+}
+
+define double @return_f64_global() {
+; CHECK-LABEL: return_f64_global:
+; CHECK-NEXT: functype   return_f64_global () -> (f64)
+; CHECK-NEXT: global.get f64_global
+; CHECK-NEXT: end_function
+  %v = load double, double addrspace(1)* @f64_global
+  ret double %v
+}
+
+
+;; LLVM doesn't yet declare proper WebAssembly globals for these values,
+;; instead placing them in linear memory.  To fix in a followup.
+; FIXME-CHECK: .globl i32_global
+; FIXME-CHECK: .globaltype i32_global, i32
+; FIXME-CHECK: .globl i64_global
+; FIXME-CHECK: .globaltype i64_global, i64
+; FIXME-CHECK: .globl f32_global
+; FIXME-CHEC

[PATCH] D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR

[Andy Wingo via Phabricator via cfe-commits]

wingo marked an inline comment as done.
wingo added a comment.

Thanks for the review!

Regarding the address space name -- the intention with "object" was to rhyme a 
bit with "object capabilities", somehow, but I see that it is confusing.

How about `WASM_ADDRESS_SPACE_MANAGED` ?  I.e. this address space is for 
"managed" variables.  Doesn't mean that the variables are under GC control 
(though they may be), but rather that the wasm implementation manages their 
storage itself, instead of having them be in linear memory.  Will post a 
renamed patch, lmk what you think.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101608/new/

https://reviews.llvm.org/D101608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR

[Andy Wingo via Phabricator via cfe-commits]

wingo added inline comments.



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISD.def:48
+
+// Reference Types
+HANDLE_MEM_NODETYPE(GLOBAL_GET)

tlively wrote:
> sbc100 wrote:
> > Is this just for ref types or also for global that hold integers too (like 
> > `__stack_pointer`)
> Yep, it's for arbitrary types (see the tests), so this comment should be 
> updated.
FWIW I think you would be able to define `__stack_pointer` from IR via
```
@__stack_pointer = external addrspace(1) global i32
```

and then to use it:

```
%current_sp = load i32, i32 addrspace(1)* @__stack_pointer
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101608/new/

https://reviews.llvm.org/D101608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR

[Andy Wingo via Phabricator via cfe-commits]

wingo updated this revision to Diff 343344.
wingo added a comment.

Rename "object" address space to "managed"; address feedback.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101608/new/

https://reviews.llvm.org/D101608

Files:
  clang/lib/Basic/Targets/WebAssembly.h
  clang/test/CodeGen/target-data.c
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h
  llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISD.def
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrRef.td
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/test/CodeGen/WebAssembly/global-get.ll
  llvm/test/CodeGen/WebAssembly/global-set.ll

Index: llvm/test/CodeGen/WebAssembly/global-set.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/global-set.ll
@@ -0,0 +1,57 @@
+; RUN: llc --mtriple=wasm32-unknown-unknown -asm-verbose=false < %s | FileCheck %s
+
+@i32_global = local_unnamed_addr addrspace(1) global i32 undef
+@i64_global = local_unnamed_addr addrspace(1) global i64 undef
+@f32_global = local_unnamed_addr addrspace(1) global float undef
+@f64_global = local_unnamed_addr addrspace(1) global double undef
+
+define void @set_i32_global(i32 %v) {
+; CHECK-LABEL: set_i32_global:
+; CHECK-NEXT: functype   set_i32_global (i32) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set i32_global
+; CHECK-NEXT: end_function
+  store i32 %v, i32 addrspace(1)* @i32_global
+  ret void
+}
+
+define void @set_i64_global(i64 %v) {
+; CHECK-LABEL: set_i64_global:
+; CHECK-NEXT: functype   set_i64_global (i64) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set i64_global
+; CHECK-NEXT: end_function
+  store i64 %v, i64 addrspace(1)* @i64_global
+  ret void
+}
+
+define void @set_f32_global(float %v) {
+; CHECK-LABEL: set_f32_global:
+; CHECK-NEXT: functype   set_f32_global (f32) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set f32_global
+; CHECK-NEXT: end_function
+  store float %v, float addrspace(1)* @f32_global
+  ret void
+}
+
+define void @set_f64_global(double %v) {
+; CHECK-LABEL: set_f64_global:
+; CHECK-NEXT: functype   set_f64_global (f64) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set f64_global
+; CHECK-NEXT: end_function
+  store double %v, double addrspace(1)* @f64_global
+  ret void
+}
+
+;; LLVM doesn't yet declare proper WebAssembly globals for these values,
+;; instead placing them in linear memory.  To fix in a followup.
+; FIXME-CHECK: .globl i32_global
+; FIXME-CHECK: .globaltype i32_global, i32
+; FIXME-CHECK: .globl i64_global
+; FIXME-CHECK: .globaltype i64_global, i64
+; FIXME-CHECK: .globl f32_global
+; FIXME-CHECK: .globaltype f32_global, f32
+; FIXME-CHECK: .globl f64_global
+; FIXME-CHECK: .globaltype f64_global, f64
Index: llvm/test/CodeGen/WebAssembly/global-get.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/global-get.ll
@@ -0,0 +1,54 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false | FileCheck %s
+
+@i32_global = local_unnamed_addr addrspace(1) global i32 undef
+@i64_global = local_unnamed_addr addrspace(1) global i64 undef
+@f32_global = local_unnamed_addr addrspace(1) global float undef
+@f64_global = local_unnamed_addr addrspace(1) global double undef
+
+define i32 @return_i32_global() {
+; CHECK-LABEL: return_i32_global:
+; CHECK-NEXT: functype   return_i32_global () -> (i32)
+; CHECK-NEXT: global.get i32_global
+; CHECK-NEXT: end_function
+  %v = load i32, i32 addrspace(1)* @i32_global
+  ret i32 %v
+}
+
+define i64 @return_i64_global() {
+; CHECK-LABEL: return_i64_global:
+; CHECK-NEXT: functype   return_i64_global () -> (i64)
+; CHECK-NEXT: global.get i64_global
+; CHECK-NEXT: end_function
+  %v = load i64, i64 addrspace(1)* @i64_global
+  ret i64 %v
+}
+
+define float @return_f32_global() {
+; CHECK-LABEL: return_f32_global:
+; CHECK-NEXT: functype   return_f32_global () -> (f32)
+; CHECK-NEXT: global.get f32_global
+; CHECK-NEXT: end_function
+  %v = load float, float addrspace(1)* @f32_global
+  ret float %v
+}
+
+define double @return_f64_global() {
+; CHECK-LABEL: return_f64_global:
+; CHECK-NEXT: functype   return_f64_global () -> (f64)
+; CHECK-NEXT: global.get f64_global
+; CHECK-NEXT: end_function
+  %v = load double, double addrspace(1)* @f64_global
+  ret double %v
+}
+
+
+;; LLVM doesn't yet declare proper WebAssembly globals for these values,
+;; instead placing them in linear memory.  To fix in a followup.
+; FIXME-CHECK: .globl i32_global
+; FIXME-CHECK: .globaltype i32_global, i32
+; FIXME-CHECK: .globl i64_global
+; FIXME-CHECK: .globaltype i64_global, i64
+; FIXME-CHECK: .globl f32_global
+; FIXME-CHEC

[PATCH] D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR

[Andy Wingo via Phabricator via cfe-commits]

wingo updated this revision to Diff 343360.
wingo added a comment.

yarr, shiver me timbers, fix me typos


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101608/new/

https://reviews.llvm.org/D101608

Files:
  clang/lib/Basic/Targets/WebAssembly.h
  clang/test/CodeGen/target-data.c
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h
  llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISD.def
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrRef.td
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/test/CodeGen/WebAssembly/global-get.ll
  llvm/test/CodeGen/WebAssembly/global-set.ll

Index: llvm/test/CodeGen/WebAssembly/global-set.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/global-set.ll
@@ -0,0 +1,57 @@
+; RUN: llc --mtriple=wasm32-unknown-unknown -asm-verbose=false < %s | FileCheck %s
+
+@i32_global = local_unnamed_addr addrspace(1) global i32 undef
+@i64_global = local_unnamed_addr addrspace(1) global i64 undef
+@f32_global = local_unnamed_addr addrspace(1) global float undef
+@f64_global = local_unnamed_addr addrspace(1) global double undef
+
+define void @set_i32_global(i32 %v) {
+; CHECK-LABEL: set_i32_global:
+; CHECK-NEXT: functype   set_i32_global (i32) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set i32_global
+; CHECK-NEXT: end_function
+  store i32 %v, i32 addrspace(1)* @i32_global
+  ret void
+}
+
+define void @set_i64_global(i64 %v) {
+; CHECK-LABEL: set_i64_global:
+; CHECK-NEXT: functype   set_i64_global (i64) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set i64_global
+; CHECK-NEXT: end_function
+  store i64 %v, i64 addrspace(1)* @i64_global
+  ret void
+}
+
+define void @set_f32_global(float %v) {
+; CHECK-LABEL: set_f32_global:
+; CHECK-NEXT: functype   set_f32_global (f32) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set f32_global
+; CHECK-NEXT: end_function
+  store float %v, float addrspace(1)* @f32_global
+  ret void
+}
+
+define void @set_f64_global(double %v) {
+; CHECK-LABEL: set_f64_global:
+; CHECK-NEXT: functype   set_f64_global (f64) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set f64_global
+; CHECK-NEXT: end_function
+  store double %v, double addrspace(1)* @f64_global
+  ret void
+}
+
+;; LLVM doesn't yet declare proper WebAssembly globals for these values,
+;; instead placing them in linear memory.  To fix in a followup.
+; FIXME-CHECK: .globl i32_global
+; FIXME-CHECK: .globaltype i32_global, i32
+; FIXME-CHECK: .globl i64_global
+; FIXME-CHECK: .globaltype i64_global, i64
+; FIXME-CHECK: .globl f32_global
+; FIXME-CHECK: .globaltype f32_global, f32
+; FIXME-CHECK: .globl f64_global
+; FIXME-CHECK: .globaltype f64_global, f64
Index: llvm/test/CodeGen/WebAssembly/global-get.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/global-get.ll
@@ -0,0 +1,54 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false | FileCheck %s
+
+@i32_global = local_unnamed_addr addrspace(1) global i32 undef
+@i64_global = local_unnamed_addr addrspace(1) global i64 undef
+@f32_global = local_unnamed_addr addrspace(1) global float undef
+@f64_global = local_unnamed_addr addrspace(1) global double undef
+
+define i32 @return_i32_global() {
+; CHECK-LABEL: return_i32_global:
+; CHECK-NEXT: functype   return_i32_global () -> (i32)
+; CHECK-NEXT: global.get i32_global
+; CHECK-NEXT: end_function
+  %v = load i32, i32 addrspace(1)* @i32_global
+  ret i32 %v
+}
+
+define i64 @return_i64_global() {
+; CHECK-LABEL: return_i64_global:
+; CHECK-NEXT: functype   return_i64_global () -> (i64)
+; CHECK-NEXT: global.get i64_global
+; CHECK-NEXT: end_function
+  %v = load i64, i64 addrspace(1)* @i64_global
+  ret i64 %v
+}
+
+define float @return_f32_global() {
+; CHECK-LABEL: return_f32_global:
+; CHECK-NEXT: functype   return_f32_global () -> (f32)
+; CHECK-NEXT: global.get f32_global
+; CHECK-NEXT: end_function
+  %v = load float, float addrspace(1)* @f32_global
+  ret float %v
+}
+
+define double @return_f64_global() {
+; CHECK-LABEL: return_f64_global:
+; CHECK-NEXT: functype   return_f64_global () -> (f64)
+; CHECK-NEXT: global.get f64_global
+; CHECK-NEXT: end_function
+  %v = load double, double addrspace(1)* @f64_global
+  ret double %v
+}
+
+
+;; LLVM doesn't yet declare proper WebAssembly globals for these values,
+;; instead placing them in linear memory.  To fix in a followup.
+; FIXME-CHECK: .globl i32_global
+; FIXME-CHECK: .globaltype i32_global, i32
+; FIXME-CHECK: .globl i64_global
+; FIXME-CHECK: .globaltype i64_global, i64
+; FIXME-CHECK: .globl f32_global
+; FIXME-CHECK: .globaltype f32_globa

[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

[Andy Wingo via Phabricator via cfe-commits]

wingo updated this revision to Diff 343367.
wingo added a comment.

Rename "object" to "managed", and add test for stack id


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101140/new/

https://reviews.llvm.org/D101140

Files:
  clang/lib/Basic/Targets/WebAssembly.h
  clang/test/CodeGen/target-data.c
  llvm/include/llvm/CodeGen/MIRYamlMapping.h
  llvm/include/llvm/CodeGen/TargetFrameLowering.h
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h
  llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyISD.def
  llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
  llvm/test/CodeGen/WebAssembly/ir-locals.ll

Index: llvm/test/CodeGen/WebAssembly/ir-locals.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals.ll
@@ -0,0 +1,76 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false | FileCheck %s
+
+%i32_cell = type i32 addrspace(1)*
+%i64_cell = type i64 addrspace(1)*
+%f32_cell = type float addrspace(1)*
+%f64_cell = type double addrspace(1)*
+
+define i32 @ir_local_i32(i32 %arg) {
+ ; CHECK-LABEL: ir_local_i32:
+ ; CHECK-NEXT: .functype ir_local_i32 (i32) -> (i32)
+ %retval = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ store i32 %arg, %i32_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ %reloaded = load i32, %i32_cell %retval
+ ; The DAG combiner infers that %reloaded is the same as %arg and
+ ; ultimately causes "local.get 0" to be emitted instead of
+ ; "local.get 1".
+ ; CHECK-NEXT: local.get 0
+ ret i32 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define i64 @ir_local_i64(i64 %arg) {
+ ; CHECK-LABEL: ir_local_i64:
+ ; CHECK-NEXT: .functype ir_local_i64 (i64) -> (i64)
+ %retval = alloca i64, addrspace(1)
+ ; CHECK-NEXT: .local i64
+ store i64 %arg, %i64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ %reloaded = load i64, %i64_cell %retval
+ ; See note in ir_local_i32.
+ ; CHECK-NEXT: local.get 0
+ ret i64 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define float @ir_local_f32(float %arg) {
+ ; CHECK-LABEL: ir_local_f32:
+ ; CHECK-NEXT: .functype ir_local_f32 (f32) -> (f32)
+ %retval = alloca float, addrspace(1)
+ ; CHECK-NEXT: .local f32
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ store float %arg, %f32_cell %retval
+ ; See note in ir_local_i32.
+ ; CHECK-NEXT: local.get 0
+ %reloaded = load float, %f32_cell %retval
+ ; CHECK-NEXT: end_function
+ ret float %reloaded
+}
+
+define double @ir_local_f64(double %arg) {
+ ; CHECK-LABEL: ir_local_f64:
+ ; CHECK-NEXT: .functype ir_local_f64 (f64) -> (f64)
+ %retval = alloca double, addrspace(1)
+ ; CHECK-NEXT: .local f64
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ store double %arg, %f64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ %reloaded = load double, %f64_cell %retval
+ ; CHECK-NEXT: end_function
+ ret double %reloaded
+}
+
+define void @ir_unreferenced_local() {
+ ; CHECK-LABEL: ir_unreferenced_local:
+ ; CHECK-NEXT: .functype ir_unreferenced_local () -> ()
+ %unused = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ ret void
+ ; CHECK-NEXT: end_function
+}
Index: llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
@@ -0,0 +1,22 @@
+; RUN: llc -mtriple=wasm32-unknown-unknown -asm-verbose=false < %s | FileCheck %s --check-prefix=CHECKCG
+; RUN: llc -mtriple=wasm32-unknown-unknown -stop-after=finalize-isel < %s | FileCheck %s --check-prefix=CHECKISEL
+
+%f32_cell = type float addrspace(1)*
+
+; CHECKISEL-LABEL: name: ir_local_f32
+; CHECKISEL:   stack:
+; CHECKISEL:   id: 0, name: retval, type: default, offset: 1, size: 1, alignment: 4,
+; CHECKISEL-NEXT:  stack-id: managed
+
+; CHECKCG-LABEL: ir_local_f32:
+; CHECKCG-NEXT: .functype ir_local_f32 (f32) -> (f32)
+; CHECKCG-NEXT: .local f32
+; CHECKCG-NEXT: local.get 0
+; CHECKCG-NEXT: local.set 1
+
+define float @ir_local_f32(float %arg) {
+ %retval = alloca float, addrspace(1)
+ store float %arg, %f32_cell %retval
+ %reloaded = load float, %f32_cell %retval
+ ret float %reloaded
+}
Index: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -120,8 +120,8 @@
 Optional CM, CodeGenOpt::Level OL, bool JIT)

[PATCH] D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR

[Andy Wingo via Phabricator via cfe-commits]

wingo added inline comments.



Comment at: llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h:39
+  // pointers are lowered to global.get / global.set or local.get / local.set,
+  // as appropriate.
+  WASM_ADDRESS_SPACE_MANAGED = 1

sunfish wrote:
> tlively wrote:
> > sunfish wrote:
> > > tlively wrote:
> > > > sunfish wrote:
> > > > > Sorry to throw more paint at the bikeshed here, but as someone who's 
> > > > > only following along at a high-level here, I found it confusing 
> > > > > whether this is talking about the wasm globals themselves, or the 
> > > > > objects referred to by reference values in the wasm globals. I think 
> > > > > the feature here is talking about the wasm globals themselves, but 
> > > > > "managed" initially made me think it might be talking about the 
> > > > > objects they reference, which in a browser context especially are 
> > > > > "managed" in every sense of the word.
> > > > Fair point. What does everyone think about `INDEXED`, because it is 
> > > > used to represent objects given static indexes (in the WebAssembly 
> > > > sense) in the final binary.
> > > Do I understand correctly that global variables and local variables are 
> > > being assigned addresses within the same conceptual address space here?
> > > 
> > > How about `WASM_VARIABLES` or `WASM_VARS`? The wasm spec terms for these 
> > > are global variables and local variables.
> > > 
> > > 
> > Sure, that works for me. We may want to rename it in the future if we end 
> > up using the same address space for tables and memories, but we can cross 
> > that bridge when we get there.
> Tables and memories have their own index spaces in wasm, so it seems like 
> they'd want their own address spaces here, perhaps 2 and 3, respectively? I 
> also agree that we can figure this out later.
`WASM_VARS` is fine with me, with one caveat -- in 
https://reviews.llvm.org/D101140  where we add support for locals, we add a new 
"core" TargetStackID enum which describes static `alloca` that is actually a 
wasm local.  Really this enum describes objects allocated on a parallel stack, 
outside of linear memory.  It was nice to use the same name to describe the 
address space and the stack id, but I don't think `variable` or `wasm-var` are 
good core enum names.  It feels like there is a core concept here for systems 
that have two stacks -- one of named objects and one of linear-memory bytes.  I 
thought `object` or `managed` could be a good name for the former, but 
evidently not :)

I will let this one sit overnight; if there are no other good ideas before 
tomorrow I will do `WASM_ADDRESS_SPACE_WASM_VARS` and either find a new name 
for `TargetStackID` or attempt to use a target-specific definition.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101608/new/

https://reviews.llvm.org/D101608

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR

[Andy Wingo via Phabricator via cfe-commits]

wingo updated this revision to Diff 344320.
wingo added a comment.

Rename "managed" address space to "wasm vars"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101608/new/

https://reviews.llvm.org/D101608

Files:
  clang/lib/Basic/Targets/WebAssembly.h
  clang/test/CodeGen/target-data.c
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h
  llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISD.def
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrRef.td
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/test/CodeGen/WebAssembly/global-get.ll
  llvm/test/CodeGen/WebAssembly/global-set.ll

Index: llvm/test/CodeGen/WebAssembly/global-set.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/global-set.ll
@@ -0,0 +1,57 @@
+; RUN: llc --mtriple=wasm32-unknown-unknown -asm-verbose=false < %s | FileCheck %s
+
+@i32_global = local_unnamed_addr addrspace(1) global i32 undef
+@i64_global = local_unnamed_addr addrspace(1) global i64 undef
+@f32_global = local_unnamed_addr addrspace(1) global float undef
+@f64_global = local_unnamed_addr addrspace(1) global double undef
+
+define void @set_i32_global(i32 %v) {
+; CHECK-LABEL: set_i32_global:
+; CHECK-NEXT: functype   set_i32_global (i32) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set i32_global
+; CHECK-NEXT: end_function
+  store i32 %v, i32 addrspace(1)* @i32_global
+  ret void
+}
+
+define void @set_i64_global(i64 %v) {
+; CHECK-LABEL: set_i64_global:
+; CHECK-NEXT: functype   set_i64_global (i64) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set i64_global
+; CHECK-NEXT: end_function
+  store i64 %v, i64 addrspace(1)* @i64_global
+  ret void
+}
+
+define void @set_f32_global(float %v) {
+; CHECK-LABEL: set_f32_global:
+; CHECK-NEXT: functype   set_f32_global (f32) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set f32_global
+; CHECK-NEXT: end_function
+  store float %v, float addrspace(1)* @f32_global
+  ret void
+}
+
+define void @set_f64_global(double %v) {
+; CHECK-LABEL: set_f64_global:
+; CHECK-NEXT: functype   set_f64_global (f64) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set f64_global
+; CHECK-NEXT: end_function
+  store double %v, double addrspace(1)* @f64_global
+  ret void
+}
+
+;; LLVM doesn't yet declare proper WebAssembly globals for these values,
+;; instead placing them in linear memory.  To fix in a followup.
+; FIXME-CHECK: .globl i32_global
+; FIXME-CHECK: .globaltype i32_global, i32
+; FIXME-CHECK: .globl i64_global
+; FIXME-CHECK: .globaltype i64_global, i64
+; FIXME-CHECK: .globl f32_global
+; FIXME-CHECK: .globaltype f32_global, f32
+; FIXME-CHECK: .globl f64_global
+; FIXME-CHECK: .globaltype f64_global, f64
Index: llvm/test/CodeGen/WebAssembly/global-get.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/global-get.ll
@@ -0,0 +1,54 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false | FileCheck %s
+
+@i32_global = local_unnamed_addr addrspace(1) global i32 undef
+@i64_global = local_unnamed_addr addrspace(1) global i64 undef
+@f32_global = local_unnamed_addr addrspace(1) global float undef
+@f64_global = local_unnamed_addr addrspace(1) global double undef
+
+define i32 @return_i32_global() {
+; CHECK-LABEL: return_i32_global:
+; CHECK-NEXT: functype   return_i32_global () -> (i32)
+; CHECK-NEXT: global.get i32_global
+; CHECK-NEXT: end_function
+  %v = load i32, i32 addrspace(1)* @i32_global
+  ret i32 %v
+}
+
+define i64 @return_i64_global() {
+; CHECK-LABEL: return_i64_global:
+; CHECK-NEXT: functype   return_i64_global () -> (i64)
+; CHECK-NEXT: global.get i64_global
+; CHECK-NEXT: end_function
+  %v = load i64, i64 addrspace(1)* @i64_global
+  ret i64 %v
+}
+
+define float @return_f32_global() {
+; CHECK-LABEL: return_f32_global:
+; CHECK-NEXT: functype   return_f32_global () -> (f32)
+; CHECK-NEXT: global.get f32_global
+; CHECK-NEXT: end_function
+  %v = load float, float addrspace(1)* @f32_global
+  ret float %v
+}
+
+define double @return_f64_global() {
+; CHECK-LABEL: return_f64_global:
+; CHECK-NEXT: functype   return_f64_global () -> (f64)
+; CHECK-NEXT: global.get f64_global
+; CHECK-NEXT: end_function
+  %v = load double, double addrspace(1)* @f64_global
+  ret double %v
+}
+
+
+;; LLVM doesn't yet declare proper WebAssembly globals for these values,
+;; instead placing them in linear memory.  To fix in a followup.
+; FIXME-CHECK: .globl i32_global
+; FIXME-CHECK: .globaltype i32_global, i32
+; FIXME-CHECK: .globl i64_global
+; FIXME-CHECK: .globaltype i64_global, i64
+; FIXME-CHECK: .globl f32_global
+; FIXME-CHECK: .globaltype f

[PATCH] D101140: [WebAssembly][CodeGen] IR support for WebAssembly local variables

[Andy Wingo via Phabricator via cfe-commits]

wingo updated this revision to Diff 344323.
wingo added a comment.

Update in response to parent commit changes


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101140/new/

https://reviews.llvm.org/D101140

Files:
  clang/lib/Basic/Targets/WebAssembly.h
  clang/test/CodeGen/target-data.c
  llvm/include/llvm/CodeGen/MIRYamlMapping.h
  llvm/include/llvm/CodeGen/TargetFrameLowering.h
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h
  llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFrameLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyISD.def
  llvm/lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
  llvm/test/CodeGen/WebAssembly/ir-locals.ll

Index: llvm/test/CodeGen/WebAssembly/ir-locals.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals.ll
@@ -0,0 +1,76 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false | FileCheck %s
+
+%i32_cell = type i32 addrspace(1)*
+%i64_cell = type i64 addrspace(1)*
+%f32_cell = type float addrspace(1)*
+%f64_cell = type double addrspace(1)*
+
+define i32 @ir_local_i32(i32 %arg) {
+ ; CHECK-LABEL: ir_local_i32:
+ ; CHECK-NEXT: .functype ir_local_i32 (i32) -> (i32)
+ %retval = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ store i32 %arg, %i32_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ %reloaded = load i32, %i32_cell %retval
+ ; The DAG combiner infers that %reloaded is the same as %arg and
+ ; ultimately causes "local.get 0" to be emitted instead of
+ ; "local.get 1".
+ ; CHECK-NEXT: local.get 0
+ ret i32 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define i64 @ir_local_i64(i64 %arg) {
+ ; CHECK-LABEL: ir_local_i64:
+ ; CHECK-NEXT: .functype ir_local_i64 (i64) -> (i64)
+ %retval = alloca i64, addrspace(1)
+ ; CHECK-NEXT: .local i64
+ store i64 %arg, %i64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ %reloaded = load i64, %i64_cell %retval
+ ; See note in ir_local_i32.
+ ; CHECK-NEXT: local.get 0
+ ret i64 %reloaded
+ ; CHECK-NEXT: end_function
+}
+
+define float @ir_local_f32(float %arg) {
+ ; CHECK-LABEL: ir_local_f32:
+ ; CHECK-NEXT: .functype ir_local_f32 (f32) -> (f32)
+ %retval = alloca float, addrspace(1)
+ ; CHECK-NEXT: .local f32
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ store float %arg, %f32_cell %retval
+ ; See note in ir_local_i32.
+ ; CHECK-NEXT: local.get 0
+ %reloaded = load float, %f32_cell %retval
+ ; CHECK-NEXT: end_function
+ ret float %reloaded
+}
+
+define double @ir_local_f64(double %arg) {
+ ; CHECK-LABEL: ir_local_f64:
+ ; CHECK-NEXT: .functype ir_local_f64 (f64) -> (f64)
+ %retval = alloca double, addrspace(1)
+ ; CHECK-NEXT: .local f64
+ ; CHECK-NEXT: local.get 0
+ ; CHECK-NEXT: local.set 1
+ store double %arg, %f64_cell %retval
+ ; CHECK-NEXT: local.get 0
+ %reloaded = load double, %f64_cell %retval
+ ; CHECK-NEXT: end_function
+ ret double %reloaded
+}
+
+define void @ir_unreferenced_local() {
+ ; CHECK-LABEL: ir_unreferenced_local:
+ ; CHECK-NEXT: .functype ir_unreferenced_local () -> ()
+ %unused = alloca i32, addrspace(1)
+ ; CHECK-NEXT: .local i32
+ ret void
+ ; CHECK-NEXT: end_function
+}
Index: llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/ir-locals-stackid.ll
@@ -0,0 +1,22 @@
+; RUN: llc -mtriple=wasm32-unknown-unknown -asm-verbose=false < %s | FileCheck %s --check-prefix=CHECKCG
+; RUN: llc -mtriple=wasm32-unknown-unknown -stop-after=finalize-isel < %s | FileCheck %s --check-prefix=CHECKISEL
+
+%f32_cell = type float addrspace(1)*
+
+; CHECKISEL-LABEL: name: ir_local_f32
+; CHECKISEL:   stack:
+; CHECKISEL:   id: 0, name: retval, type: default, offset: 1, size: 1, alignment: 4,
+; CHECKISEL-NEXT:  stack-id: wasm-local
+
+; CHECKCG-LABEL: ir_local_f32:
+; CHECKCG-NEXT: .functype ir_local_f32 (f32) -> (f32)
+; CHECKCG-NEXT: .local f32
+; CHECKCG-NEXT: local.get 0
+; CHECKCG-NEXT: local.set 1
+
+define float @ir_local_f32(float %arg) {
+ %retval = alloca float, addrspace(1)
+ store float %arg, %f32_cell %retval
+ %reloaded = load float, %f32_cell %retval
+ ret float %reloaded
+}
Index: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -120,8 +120,8 @@
 Optional CM, CodeGenOpt::Level OL, bool JIT)
 : L

[PATCH] D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR

[Andy Wingo via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd7086af2143d: [WebAssembly] Support for WebAssembly globals 
in LLVM IR (authored by pmatos, committed by wingo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101608/new/

https://reviews.llvm.org/D101608

Files:
  clang/lib/Basic/Targets/WebAssembly.h
  clang/test/CodeGen/target-data.c
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h
  llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISD.def
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrRef.td
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/test/CodeGen/WebAssembly/global-get.ll
  llvm/test/CodeGen/WebAssembly/global-set.ll

Index: llvm/test/CodeGen/WebAssembly/global-set.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/global-set.ll
@@ -0,0 +1,57 @@
+; RUN: llc --mtriple=wasm32-unknown-unknown -asm-verbose=false < %s | FileCheck %s
+
+@i32_global = local_unnamed_addr addrspace(1) global i32 undef
+@i64_global = local_unnamed_addr addrspace(1) global i64 undef
+@f32_global = local_unnamed_addr addrspace(1) global float undef
+@f64_global = local_unnamed_addr addrspace(1) global double undef
+
+define void @set_i32_global(i32 %v) {
+; CHECK-LABEL: set_i32_global:
+; CHECK-NEXT: functype   set_i32_global (i32) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set i32_global
+; CHECK-NEXT: end_function
+  store i32 %v, i32 addrspace(1)* @i32_global
+  ret void
+}
+
+define void @set_i64_global(i64 %v) {
+; CHECK-LABEL: set_i64_global:
+; CHECK-NEXT: functype   set_i64_global (i64) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set i64_global
+; CHECK-NEXT: end_function
+  store i64 %v, i64 addrspace(1)* @i64_global
+  ret void
+}
+
+define void @set_f32_global(float %v) {
+; CHECK-LABEL: set_f32_global:
+; CHECK-NEXT: functype   set_f32_global (f32) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set f32_global
+; CHECK-NEXT: end_function
+  store float %v, float addrspace(1)* @f32_global
+  ret void
+}
+
+define void @set_f64_global(double %v) {
+; CHECK-LABEL: set_f64_global:
+; CHECK-NEXT: functype   set_f64_global (f64) -> ()
+; CHECK-NEXT: local.get 0
+; CHECK-NEXT: global.set f64_global
+; CHECK-NEXT: end_function
+  store double %v, double addrspace(1)* @f64_global
+  ret void
+}
+
+;; LLVM doesn't yet declare proper WebAssembly globals for these values,
+;; instead placing them in linear memory.  To fix in a followup.
+; FIXME-CHECK: .globl i32_global
+; FIXME-CHECK: .globaltype i32_global, i32
+; FIXME-CHECK: .globl i64_global
+; FIXME-CHECK: .globaltype i64_global, i64
+; FIXME-CHECK: .globl f32_global
+; FIXME-CHECK: .globaltype f32_global, f32
+; FIXME-CHECK: .globl f64_global
+; FIXME-CHECK: .globaltype f64_global, f64
Index: llvm/test/CodeGen/WebAssembly/global-get.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/global-get.ll
@@ -0,0 +1,54 @@
+; RUN: llc < %s --mtriple=wasm32-unknown-unknown -asm-verbose=false | FileCheck %s
+
+@i32_global = local_unnamed_addr addrspace(1) global i32 undef
+@i64_global = local_unnamed_addr addrspace(1) global i64 undef
+@f32_global = local_unnamed_addr addrspace(1) global float undef
+@f64_global = local_unnamed_addr addrspace(1) global double undef
+
+define i32 @return_i32_global() {
+; CHECK-LABEL: return_i32_global:
+; CHECK-NEXT: functype   return_i32_global () -> (i32)
+; CHECK-NEXT: global.get i32_global
+; CHECK-NEXT: end_function
+  %v = load i32, i32 addrspace(1)* @i32_global
+  ret i32 %v
+}
+
+define i64 @return_i64_global() {
+; CHECK-LABEL: return_i64_global:
+; CHECK-NEXT: functype   return_i64_global () -> (i64)
+; CHECK-NEXT: global.get i64_global
+; CHECK-NEXT: end_function
+  %v = load i64, i64 addrspace(1)* @i64_global
+  ret i64 %v
+}
+
+define float @return_f32_global() {
+; CHECK-LABEL: return_f32_global:
+; CHECK-NEXT: functype   return_f32_global () -> (f32)
+; CHECK-NEXT: global.get f32_global
+; CHECK-NEXT: end_function
+  %v = load float, float addrspace(1)* @f32_global
+  ret float %v
+}
+
+define double @return_f64_global() {
+; CHECK-LABEL: return_f64_global:
+; CHECK-NEXT: functype   return_f64_global () -> (f64)
+; CHECK-NEXT: global.get f64_global
+; CHECK-NEXT: end_function
+  %v = load double, double addrspace(1)* @f64_global
+  ret double %v
+}
+
+
+;; LLVM doesn't yet declare proper WebAssembly globals for these values,
+;; instead placing them in linear memory.  To fix in a followup.
+; FIXME-CHECK: .globl i32_global
+; FIXME-CHECK: .globaltype i32_globa

[PATCH] D108445: [clang][NFC] GetOrCreateLLVMGlobal takes LangAS

[Andy Wingo via Phabricator via cfe-commits]

wingo created this revision.
wingo requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Pass a LangAS instead of a target address space to
GetOrCreateLLVMGlobal, to remove a place where the frontend assumes that
target address space 0 is special.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108445

Files:
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h

Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1478,8 +1478,8 @@
   void UpdateMultiVersionNames(GlobalDecl GD, const FunctionDecl *FD);
 
   llvm::Constant *
-  GetOrCreateLLVMGlobal(StringRef MangledName, llvm::Type *Ty,
-unsigned AddrSpace, const VarDecl *D,
+  GetOrCreateLLVMGlobal(StringRef MangledName, llvm::Type *Ty, LangAS AddrSpace,
+const VarDecl *D,
 ForDefinition_t IsForDefinition = NotForDefinition);
 
   bool GetCPUAndFeaturesAttributes(GlobalDecl GD,
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2851,7 +2851,8 @@
   GlobalDecl(cast(VD)),
   /*ForVTable=*/false);
   else
-Aliasee = GetOrCreateLLVMGlobal(AA->getAliasee(), DeclTy, 0, nullptr);
+Aliasee = GetOrCreateLLVMGlobal(AA->getAliasee(), DeclTy, LangAS::Default,
+nullptr);
 
   auto *F = cast(Aliasee);
   F->setLinkage(llvm::Function::ExternalWeakLinkage);
@@ -3824,10 +3825,11 @@
 /// mangled name but some other type.
 llvm::Constant *
 CodeGenModule::GetOrCreateLLVMGlobal(StringRef MangledName, llvm::Type *Ty,
- unsigned AddrSpace, const VarDecl *D,
+ LangAS AddrSpace, const VarDecl *D,
  ForDefinition_t IsForDefinition) {
   // Lookup the entry, lazily creating it if necessary.
   llvm::GlobalValue *Entry = GetGlobalValue(MangledName);
+  unsigned TargetAS = getContext().getTargetAddressSpace(AddrSpace);
   if (Entry) {
 if (WeakRefReferences.erase(Entry)) {
   if (D && !D->hasAttr())
@@ -3841,7 +3843,7 @@
 if (LangOpts.OpenMP && !LangOpts.OpenMPSimd && D)
   getOpenMPRuntime().registerTargetGlobalVariable(D, Entry);
 
-if (Entry->getValueType() == Ty && Entry->getAddressSpace() == AddrSpace)
+if (Entry->getValueType() == Ty && Entry->getAddressSpace() == TargetAS)
   return Entry;
 
 // If there are two attempts to define the same mangled name, issue an
@@ -3865,24 +3867,23 @@
 }
 
 // Make sure the result is of the correct type.
-if (Entry->getType()->getAddressSpace() != AddrSpace) {
+if (Entry->getType()->getAddressSpace() != TargetAS) {
   return llvm::ConstantExpr::getAddrSpaceCast(Entry,
-  Ty->getPointerTo(AddrSpace));
+  Ty->getPointerTo(TargetAS));
 }
 
 // (If global is requested for a definition, we always need to create a new
 // global, not just return a bitcast.)
 if (!IsForDefinition)
-  return llvm::ConstantExpr::getBitCast(Entry, Ty->getPointerTo(AddrSpace));
+  return llvm::ConstantExpr::getBitCast(Entry, Ty->getPointerTo(TargetAS));
   }
 
   auto DAddrSpace = GetGlobalVarAddressSpace(D);
-  auto TargetAddrSpace = getContext().getTargetAddressSpace(DAddrSpace);
 
   auto *GV = new llvm::GlobalVariable(
   getModule(), Ty, false, llvm::GlobalValue::ExternalLinkage, nullptr,
   MangledName, nullptr, llvm::GlobalVariable::NotThreadLocal,
-  TargetAddrSpace);
+  getContext().getTargetAddressSpace(DAddrSpace));
 
   // If we already created a global with the same mangled name (but different
   // type) before, take its name and remove it from its parent.
@@ -4005,10 +4006,10 @@
   LangAS ExpectedAS =
   D ? D->getType().getAddressSpace()
 : (LangOpts.OpenCL ? LangAS::opencl_global : LangAS::Default);
-  assert(getContext().getTargetAddressSpace(ExpectedAS) == AddrSpace);
+  assert(getContext().getTargetAddressSpace(ExpectedAS) == TargetAS);
   if (DAddrSpace != ExpectedAS) {
 return getTargetCodeGenInfo().performAddrSpaceCast(
-*this, GV, DAddrSpace, ExpectedAS, Ty->getPointerTo(AddrSpace));
+*this, GV, DAddrSpace, ExpectedAS, Ty->getPointerTo(TargetAS));
   }
 
   return GV;
@@ -4098,8 +4099,7 @@
 Ty = getTypes().ConvertTypeForMem(ASTTy);
 
   StringRef MangledName = getMangledName(D);
-  return GetOrCreateLLVMGlobal(MangledName, Ty,
-   getContext().getTargetAddressSpace(ASTTy), D,
+  return GetOrCreateLLVMGlobal(Ma

[PATCH] D108449: [clang][NFC] Tighten up code for GetGlobalVarAddressSpace

[Andy Wingo via Phabricator via cfe-commits]

wingo created this revision.
Herald added a subscriber: Anastasia.
wingo requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The LangAS local is only used in the OpenCL case; move its decl
inwards.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108449

Files:
  clang/lib/CodeGen/CodeGenModule.cpp


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4148,16 +4148,15 @@
 }
 
 LangAS CodeGenModule::GetGlobalVarAddressSpace(const VarDecl *D) {
-  LangAS AddrSpace = LangAS::Default;
   if (LangOpts.OpenCL) {
-AddrSpace = D ? D->getType().getAddressSpace() : LangAS::opencl_global;
-assert(AddrSpace == LangAS::opencl_global ||
-   AddrSpace == LangAS::opencl_global_device ||
-   AddrSpace == LangAS::opencl_global_host ||
-   AddrSpace == LangAS::opencl_constant ||
-   AddrSpace == LangAS::opencl_local ||
-   AddrSpace >= LangAS::FirstTargetAddressSpace);
-return AddrSpace;
+LangAS AS = D ? D->getType().getAddressSpace() : LangAS::opencl_global;
+assert(AS == LangAS::opencl_global ||
+   AS == LangAS::opencl_global_device ||
+   AS == LangAS::opencl_global_host ||
+   AS == LangAS::opencl_constant ||
+   AS == LangAS::opencl_local ||
+   AS >= LangAS::FirstTargetAddressSpace);
+return AS;
   }
 
   if (LangOpts.SYCLIsDevice &&


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4148,16 +4148,15 @@
 }
 
 LangAS CodeGenModule::GetGlobalVarAddressSpace(const VarDecl *D) {
-  LangAS AddrSpace = LangAS::Default;
   if (LangOpts.OpenCL) {
-AddrSpace = D ? D->getType().getAddressSpace() : LangAS::opencl_global;
-assert(AddrSpace == LangAS::opencl_global ||
-   AddrSpace == LangAS::opencl_global_device ||
-   AddrSpace == LangAS::opencl_global_host ||
-   AddrSpace == LangAS::opencl_constant ||
-   AddrSpace == LangAS::opencl_local ||
-   AddrSpace >= LangAS::FirstTargetAddressSpace);
-return AddrSpace;
+LangAS AS = D ? D->getType().getAddressSpace() : LangAS::opencl_global;
+assert(AS == LangAS::opencl_global ||
+   AS == LangAS::opencl_global_device ||
+   AS == LangAS::opencl_global_host ||
+   AS == LangAS::opencl_constant ||
+   AS == LangAS::opencl_local ||
+   AS >= LangAS::FirstTargetAddressSpace);
+return AS;
   }
 
   if (LangOpts.SYCLIsDevice &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108449: [clang][NFC] Tighten up code for GetGlobalVarAddressSpace

[Andy Wingo via Phabricator via cfe-commits]

wingo added a reviewer: rjmccall.
wingo added a comment.

Just a drive-by cleanup while poking other address-space-related things.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108449/new/

https://reviews.llvm.org/D108449

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108450: [clang][CodeGen] GetDefaultAlignTempAlloca uses preferred alignment

[Andy Wingo via Phabricator via cfe-commits]

wingo created this revision.
wingo added a reviewer: rjmccall.
wingo requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This function was defaulting to use the ABI alignment for the LLVM
type.  Here we change to use the preferred alignment.  This will allow
unification with GetTempAlloca, which if alignment isn't specified, uses
the preferred alignment.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108450

Files:
  clang/lib/CodeGen/CGExpr.cpp


Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -122,7 +122,7 @@
 Address CodeGenFunction::CreateDefaultAlignTempAlloca(llvm::Type *Ty,
   const Twine &Name) {
   CharUnits Align =
-CharUnits::fromQuantity(CGM.getDataLayout().getABITypeAlignment(Ty));
+  CharUnits::fromQuantity(CGM.getDataLayout().getPrefTypeAlignment(Ty));
   return CreateTempAlloca(Ty, Align, Name);
 }
 


Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -122,7 +122,7 @@
 Address CodeGenFunction::CreateDefaultAlignTempAlloca(llvm::Type *Ty,
   const Twine &Name) {
   CharUnits Align =
-CharUnits::fromQuantity(CGM.getDataLayout().getABITypeAlignment(Ty));
+  CharUnits::fromQuantity(CGM.getDataLayout().getPrefTypeAlignment(Ty));
   return CreateTempAlloca(Ty, Align, Name);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108450: [clang][CodeGen] GetDefaultAlignTempAlloca uses preferred alignment

[Andy Wingo via Phabricator via cfe-commits]

wingo added a comment.

I tested by building with all targets enabled and there was no change in test 
results, running clang, llvm, and lld tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108450/new/

https://reviews.llvm.org/D108450

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108458: [clang][CodeGen] Add default constructor for Address. NFC.

[Andy Wingo via Phabricator via cfe-commits]

wingo created this revision.
wingo added a reviewer: rjmccall.
wingo requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This allows a declaration of an Address to be invalid by default,
allowing it to be added to e.g. a DenseMap.  Will followup with a
cleanup.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108458

Files:
  clang/lib/CodeGen/Address.h


Index: clang/lib/CodeGen/Address.h
===
--- clang/lib/CodeGen/Address.h
+++ clang/lib/CodeGen/Address.h
@@ -22,16 +22,17 @@
 
 /// An aligned address.
 class Address {
-  llvm::Value *Pointer;
+  llvm::Value *Pointer = nullptr;
   CharUnits Alignment;
 public:
+  Address() { assert(!isValid()); }
   Address(llvm::Value *pointer, CharUnits alignment)
   : Pointer(pointer), Alignment(alignment) {
 assert((!alignment.isZero() || pointer == nullptr) &&
"creating valid address with invalid alignment");
   }
 
-  static Address invalid() { return Address(nullptr, CharUnits()); }
+  static Address invalid() { return Address(); }
   bool isValid() const { return Pointer != nullptr; }
 
   llvm::Value *getPointer() const {


Index: clang/lib/CodeGen/Address.h
===
--- clang/lib/CodeGen/Address.h
+++ clang/lib/CodeGen/Address.h
@@ -22,16 +22,17 @@
 
 /// An aligned address.
 class Address {
-  llvm::Value *Pointer;
+  llvm::Value *Pointer = nullptr;
   CharUnits Alignment;
 public:
+  Address() { assert(!isValid()); }
   Address(llvm::Value *pointer, CharUnits alignment)
   : Pointer(pointer), Alignment(alignment) {
 assert((!alignment.isZero() || pointer == nullptr) &&
"creating valid address with invalid alignment");
   }
 
-  static Address invalid() { return Address(nullptr, CharUnits()); }
+  static Address invalid() { return Address(); }
   bool isValid() const { return Pointer != nullptr; }
 
   llvm::Value *getPointer() const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108458: [clang][CodeGen] Add default constructor for Address. NFC.

[Andy Wingo via Phabricator via cfe-commits]

wingo added inline comments.



Comment at: clang/lib/CodeGen/Address.h:31
   : Pointer(pointer), Alignment(alignment) {
 assert((!alignment.isZero() || pointer == nullptr) &&
"creating valid address with invalid alignment");

It would be nice to assert(isValid()) here but that's not how Address is used; 
you can pass in a nullptr as the first argument.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108458/new/

https://reviews.llvm.org/D108458

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108459: [clang][CodeGen] Rely on implicitly invalid Address. NFC.

[Andy Wingo via Phabricator via cfe-commits]

wingo created this revision.
Herald added a subscriber: lxfind.
wingo requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Remove explicit uses of Address::invalid in initializers; the default
constructor makes an invalid address.

Depends on D108458 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108459

Files:
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGCall.h
  clang/lib/CodeGen/CGCoroutine.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/TargetInfo.cpp

Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -321,7 +321,7 @@
   llvm::Value *Ptr = CGF.Builder.CreateLoad(VAListAddr, "argp.cur");
 
   // If the CC aligns values higher than the slot size, do so if needed.
-  Address Addr = Address::invalid();
+  Address Addr;
   if (AllowHigherAlign && DirectAlign > SlotSize) {
 Addr = Address(emitRoundPointerUpToAlignment(CGF, Ptr, DirectAlign),
  DirectAlign);
@@ -4064,7 +4064,7 @@
   // register save space).
 
   llvm::Value *InRegs = nullptr;
-  Address gp_offset_p = Address::invalid(), fp_offset_p = Address::invalid();
+  Address gp_offset_p, fp_offset_p;
   llvm::Value *gp_offset = nullptr, *fp_offset = nullptr;
   if (neededInt) {
 gp_offset_p = CGF.Builder.CreateStructGEP(VAListAddr, 0, "gp_offset_p");
@@ -4105,7 +4105,7 @@
   llvm::Value *RegSaveArea = CGF.Builder.CreateLoad(
   CGF.Builder.CreateStructGEP(VAListAddr, 3), "reg_save_area");
 
-  Address RegAddr = Address::invalid();
+  Address RegAddr;
   if (neededInt && neededSSE) {
 // FIXME: Cleanup.
 assert(AI.isDirect() && "Unexpected ABI info for mixed regs");
@@ -4787,7 +4787,7 @@
   CGBuilderTy &Builder = CGF.Builder;
 
   // The calling convention either uses 1-2 GPRs or 1 FPR.
-  Address NumRegsAddr = Address::invalid();
+  Address NumRegsAddr;
   if (isInt || IsSoftFloatABI) {
 NumRegsAddr = Builder.CreateStructGEP(VAList, 0, "gpr");
   } else {
@@ -4815,7 +4815,7 @@
   if (isIndirect) DirectTy = DirectTy->getPointerTo(0);
 
   // Case 1: consume registers.
-  Address RegAddr = Address::invalid();
+  Address RegAddr;
   {
 CGF.EmitBlock(UsingRegs);
 
@@ -4850,7 +4850,7 @@
   }
 
   // Case 2: consume space in the overflow area.
-  Address MemAddr = Address::invalid();
+  Address MemAddr;
   {
 CGF.EmitBlock(UsingOverflow);
 
@@ -5982,7 +5982,7 @@
   CharUnits TySize = getContext().getTypeSizeInChars(Ty);
   CharUnits TyAlign = getContext().getTypeUnadjustedAlignInChars(Ty);
 
-  Address reg_offs_p = Address::invalid();
+  Address reg_offs_p;
   llvm::Value *reg_offs = nullptr;
   int reg_top_index;
   int RegSize = IsIndirect ? 8 : TySize.getQuantity();
@@ -6063,7 +6063,7 @@
   reg_top = CGF.Builder.CreateLoad(reg_top_p, "reg_top");
   Address BaseAddr(CGF.Builder.CreateInBoundsGEP(CGF.Int8Ty, reg_top, reg_offs),
CharUnits::fromQuantity(IsFPR ? 16 : 8));
-  Address RegAddr = Address::invalid();
+  Address RegAddr;
   llvm::Type *MemTy = CGF.ConvertTypeForMem(Ty);
 
   if (IsIndirect) {
@@ -9636,7 +9636,7 @@
 
   auto TypeInfo = getContext().getTypeInfoInChars(Ty);
 
-  Address ArgAddr = Address::invalid();
+  Address ArgAddr;
   CharUnits Stride;
   switch (AI.getKind()) {
   case ABIArgInfo::Expand:
@@ -10006,7 +10006,7 @@
 AI.setCoerceToType(ArgTy);
   llvm::Type *ArgPtrTy = llvm::PointerType::getUnqual(ArgTy);
 
-  Address Val = Address::invalid();
+  Address Val;
   CharUnits ArgSize = CharUnits::Zero();
   switch (AI.getKind()) {
   case ABIArgInfo::Expand:
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -354,11 +354,11 @@
 
   /// ReturnValue - The temporary alloca to hold the return
   /// value. This is invalid iff the function has no return value.
-  Address ReturnValue = Address::invalid();
+  Address ReturnValue;
 
   /// ReturnValuePointer - The temporary alloca to hold a pointer to sret.
   /// This is invalid if sret is not in use.
-  Address ReturnValueP

[PATCH] D108464: [clang][CodeGen] Refactor CreateTempAlloca function nest. NFC.

[Andy Wingo via Phabricator via cfe-commits]

wingo created this revision.
wingo added a reviewer: rjmccall.
Herald added subscribers: lxfind, sunfish, dschuff.
wingo requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, aheejin.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

It used to be that there were three layers to create temp alloca
instructions in CodeGenFunction.  The lowest level was named
CreateTempAlloca and returned an LLVM instruction (1):

  llvm::AllocaInst* CreateTempAlloca(llvm::Type *Ty);

(Leaving off the name argument and array size from the prototype, for
brevity.)

The next level applied frontend-specified alignment to the alloca and
returned an address, but left the value in the alloca address space (2):

  Address
  CreateTempAllocaWithoutCast(llvm::Type *Ty, CharUnits Align);

Finally the normal function returns an Address, but also makes sure that
the result is in LangAS::Default (3):

  Address
  CreateTempAlloca(QualType Ty, CharUnits Align);

This is a bit confusing since functions (1) and (3) share a name but
have different behavior, and function (2) has a funny name.
Furthermore, the implementation of function (2) actually calls
function (1), making it seem to the reader like there is a loop in the
call graph.

This patch refactors to remove function (1) and replace code that uses
it with calls to function (2), returning an Address instead of an IR
instruction.  This also removes some places in which the frontend wasn't
specifying the alignment of its allocas.

This patch also changes function (2) to explicitly take an address space
argument, which should generally be the alloca address space.  There is
usually one target-specified alloca address space, but in the future,
the WebAssembly target may alloca variables in multiple address spaces.
The function name is changed from CreateTempAllocaWithoutCast to
CreateTempAllocaInAS, indicating that the result is left in the given
AS.

Finally, we also replace uses of the somewhat-deprecated
CreateDefaultAlignTempAlloca with CreateTempAllocaInAS, passing the
result of calling the new CodeGenFunction::PreferredAlignmentForIRType
method as the alignnment.

As a result of this patch, a number of llvm::Value* variables are
changed to be Address instead.  This allows a more simplified codegen,
as the IR builder doesn't need to take an additional alignment argument.

Depends on D108459 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108464

Files:
  clang/lib/CodeGen/CGBuilder.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGCleanup.cpp
  clang/lib/CodeGen/CGCoroutine.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CGGPUBuiltin.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h

Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -576,7 +576,7 @@
 
   /// A mapping from NRVO variables to the flags used to indicate
   /// when the NRVO has been applied to this variable.
-  llvm::DenseMap NRVOFlags;
+  llvm::DenseMap NRVOFlags;
 
   EHScopeStack EHStack;
   llvm::SmallVector LifetimeExtendedCleanupStack;
@@ -623,11 +623,11 @@
 
   /// The exception slot.  All landing pads write the current exception pointer
   /// into this alloca.
-  llvm::Value *ExceptionSlot = nullptr;
+  Address ExceptionSlot;
 
   /// The selector slot.  Under the MandatoryCleanup model, all landing pads
   /// write the current selector value into this alloca.
-  llvm::AllocaInst *EHSelectorSlot = nullptr;
+  Address EHSelectorSlot;
 
   /// A stack of exception code slots. Entering an __except block pushes a slot
   /// on the stack and leaving pops one. The __exception_code() intrinsic loads
@@ -704,11 +704,11 @@
 
 /// An i1 variable indicating whether or not the @finally is
 /// running for an exception.
-llvm::AllocaInst *ForEHVar;
+Address ForEHVar;
 
 /// An i8* variable into which the exception pointer to rethrow
 /// has been saved.
-llvm::AllocaInst *SavedExnVar;
+Address SavedExnVar;
 
   public:
 void enter(CodeGenFunction &CGF, const Stmt *Finally,
@@ -2470,54 +2470,58 @@
 TBAAAccessInfo *TBAAInfo = nullptr);
   LValue EmitLoadOfPointerLValue(Address Ptr, const PointerType *PtrTy);
 
-  /// CreateTempAlloca - This creates an alloca and inserts it into the entry
-  /// block if \p ArraySize is nullptr, otherwise inserts it at the current
-  /// insertion point of the builder. The caller is responsible for setting an
-  /// appropriate alignment on
-  /// the alloca.
+  /// CreateTempAllocaInAS - Create 

[PATCH] D108464: [clang][CodeGen] Refactor CreateTempAlloca function nest. NFC.

[Andy Wingo via Phabricator via cfe-commits]

wingo added a comment.

So... besides the refactor, this is getting closer to where I'm going in 
https://lists.llvm.org/pipermail/cfe-dev/2021-July/068559.html, though still 
NFC.  I think you can see where I would replace `getASTAllocaAddressSpace` with 
`getAllocaAddressSpace(QualType Ty)`, and possibly (depending on the source 
language) avoid casting the resulting alloca to `LangAS::Default`.  WDYT, is 
this sort of thing OK?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108464/new/

https://reviews.llvm.org/D108464

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108464: [clang][CodeGen] Refactor CreateTempAlloca function nest. NFC.

[Andy Wingo via Phabricator via cfe-commits]

wingo added inline comments.



Comment at: clang/lib/CodeGen/CGBuilder.h:115
 
-  /// Emit a load from an i1 flag variable.
-  llvm::LoadInst *CreateFlagLoad(llvm::Value *Addr,

it's the change to always return an `Address` from `CreateTempAlloca` that 
makes these methods unnecessary.



Comment at: clang/lib/CodeGen/CGGPUBuiltin.cpp:116
   llvm::Value *Arg = Args[I].getRValue(*this).getScalarVal();
-  Builder.CreateAlignedStore(Arg, P, DL.getPrefTypeAlign(Arg->getType()));
+  // FIXME: Changing the following line to Builder.CreateStore(Arg, P)
+  // results in a test failure in OpenMP/nvptx_target_printf_codegen, in

this is an open question -- there could be a bug here in the existing code.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108464/new/

https://reviews.llvm.org/D108464

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108464: [clang][CodeGen] Refactor CreateTempAlloca function nest. NFC.

[Andy Wingo via Phabricator via cfe-commits]

wingo added a subscriber: tlively.
wingo added a comment.

In D108464#2957276 , @wingo wrote:

> So... besides the refactor, this is getting closer to where I'm going in 
> https://lists.llvm.org/pipermail/cfe-dev/2021-July/068559.html, though still 
> NFC.  I think you can see where I would replace `getASTAllocaAddressSpace` 
> with `getAllocaAddressSpace(QualType Ty)`, and possibly (depending on the 
> source language) avoid casting the resulting alloca to `LangAS::Default`.  
> WDYT, is this sort of thing OK?

Taking this patch as perhaps a better generic discussion point, @rjmccall 
graciously gave some general feedback on this approach (thank you!!!):

In D108360#2957844 , @rjmccall wrote:

> I'm not sure that I agree with your overall plan, though:
>
> - The WebAssembly operand stack is not a good match for an address space at 
> the language level because it's not addressable at all.  If you can't 
> meaningfully have a pointer into the address space, then you don't really 
> need this in the type system; it's more like a declaration modifier at best.
> - Allocating local variables on the operand stack ought to be a very 
> straightforward analysis in the backend.  There's not much optimization value 
> in trying to do it in the frontend, and it's going to be problematic for 
> things like coroutine lowering.
> - The security argument seems pretty weak, not because security isn't 
> important but because this is not really an adequate basis for getting the 
> tamper-proof guarantee you want.  For example, LLVM passes can and do 
> introduce its own allocas and store scalars into them sometimes.  Really you 
> need some sort of "tamper-proof" *type* which the compiler can make an 
> end-to-end guarantee of non-tamper-ability for the values of, and while 
> optimally this would be implemented by just keeping values on the operand 
> stack, in the general case you will need to have some sort of strategy for 
> keeping things in memory.

Thanks for thinking about this!  Indeed I started out with the goal of not 
going deep into clang and if it's possible to avoid going too deeply, that 
would be better for everyone involved.  I am starting to think however that it 
may be unavoidable for me at least.

So, I am focusing on WebAssembly global and local variables; the WebAssembly 
operand stack is an artifact of the IR-to-MC lowering and AFAICS doesn't have 
any bearing on what clang does -- though perhaps I am misunderstanding what you 
are getting at here.  The issue is not to allocate locals on the operand stack, 
but rather to allocate them as part of the "locals" of a WebAssembly function 
.  Cc 
@tlively on the WebAssembly side.

I agree that the security argument is weak: it's something but it's not the 
real motivation.

The main motivator is the ability to have "reference type" 
 
(`externref`/`funcref`) locals and globals 
 at all.  
Reference-typed values can't be stored to linear memory.  They have no size and 
no byte representation -- they are opaque values from the host.  However, 
WebAssembly locals and globals can define storage locations of type `externref` 
or `funcref`.  The storage locations for WebAssembly locals and globals are not 
in linear memory, and are not addressable by pointer at run-time -- accesses to 
them are always by immediate.

Currently, clang always produces LLVM IR that allocates C++ globals and locals 
in linear memory.  LLVM may transform some of these to WebAssembly globals or 
locals at its discretion.  This strategy works because all values for the 
initial set of types supported by WebAssembly can be stored to linear memory; 
what you can do with a WebAssembly global or local was a subset of what you 
could do with linear-memory globals and alloca locals.

However, with reference types (merged into the spec earlier this year), this is 
no longer the case -- there are now types representable in WebAssembly 
globals/locals which can't be represented in linear memory.

Because of the limitations in how WebAssembly globals and locals can be used, 
reference-typed values have associated semantic restrictions in the front-end.  
If I declare a C++ local of type externref (which must be allocated to a 
WebAssembly local), I can't take its address:

  void f() {
externref_t x = g();
h(&x); // error
  }

Similarly I can't put an externref in an aggregate type that itself is 
allocated in linear memory:

  // global
  struct { int x; externref_t y; } z; // error

But, if we add a generic OpenCL-like address space attribute, that would allow 
the user to declare some variables to be in alternate address spaces.  Then we 
can apply the ACLE SVE semantic restrictions 

[PATCH] D108360: [clang][NFC] Remove dead code

[Andy Wingo via Phabricator via cfe-commits]

wingo added a subscriber: tlively.
wingo added a comment.

Thanks for feedback!  Following up on general question in D108464 
.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108360/new/

https://reviews.llvm.org/D108360

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108445: [clang][NFC] GetOrCreateLLVMGlobal takes LangAS

[Andy Wingo via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd3d4d98576f4: [clang][NFC] GetOrCreateLLVMGlobal takes 
LangAS (authored by wingo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108445/new/

https://reviews.llvm.org/D108445

Files:
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h

Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1478,8 +1478,8 @@
   void UpdateMultiVersionNames(GlobalDecl GD, const FunctionDecl *FD);
 
   llvm::Constant *
-  GetOrCreateLLVMGlobal(StringRef MangledName, llvm::Type *Ty,
-unsigned AddrSpace, const VarDecl *D,
+  GetOrCreateLLVMGlobal(StringRef MangledName, llvm::Type *Ty, LangAS AddrSpace,
+const VarDecl *D,
 ForDefinition_t IsForDefinition = NotForDefinition);
 
   bool GetCPUAndFeaturesAttributes(GlobalDecl GD,
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2851,7 +2851,8 @@
   GlobalDecl(cast(VD)),
   /*ForVTable=*/false);
   else
-Aliasee = GetOrCreateLLVMGlobal(AA->getAliasee(), DeclTy, 0, nullptr);
+Aliasee = GetOrCreateLLVMGlobal(AA->getAliasee(), DeclTy, LangAS::Default,
+nullptr);
 
   auto *F = cast(Aliasee);
   F->setLinkage(llvm::Function::ExternalWeakLinkage);
@@ -3824,10 +3825,11 @@
 /// mangled name but some other type.
 llvm::Constant *
 CodeGenModule::GetOrCreateLLVMGlobal(StringRef MangledName, llvm::Type *Ty,
- unsigned AddrSpace, const VarDecl *D,
+ LangAS AddrSpace, const VarDecl *D,
  ForDefinition_t IsForDefinition) {
   // Lookup the entry, lazily creating it if necessary.
   llvm::GlobalValue *Entry = GetGlobalValue(MangledName);
+  unsigned TargetAS = getContext().getTargetAddressSpace(AddrSpace);
   if (Entry) {
 if (WeakRefReferences.erase(Entry)) {
   if (D && !D->hasAttr())
@@ -3841,7 +3843,7 @@
 if (LangOpts.OpenMP && !LangOpts.OpenMPSimd && D)
   getOpenMPRuntime().registerTargetGlobalVariable(D, Entry);
 
-if (Entry->getValueType() == Ty && Entry->getAddressSpace() == AddrSpace)
+if (Entry->getValueType() == Ty && Entry->getAddressSpace() == TargetAS)
   return Entry;
 
 // If there are two attempts to define the same mangled name, issue an
@@ -3865,24 +3867,23 @@
 }
 
 // Make sure the result is of the correct type.
-if (Entry->getType()->getAddressSpace() != AddrSpace) {
+if (Entry->getType()->getAddressSpace() != TargetAS) {
   return llvm::ConstantExpr::getAddrSpaceCast(Entry,
-  Ty->getPointerTo(AddrSpace));
+  Ty->getPointerTo(TargetAS));
 }
 
 // (If global is requested for a definition, we always need to create a new
 // global, not just return a bitcast.)
 if (!IsForDefinition)
-  return llvm::ConstantExpr::getBitCast(Entry, Ty->getPointerTo(AddrSpace));
+  return llvm::ConstantExpr::getBitCast(Entry, Ty->getPointerTo(TargetAS));
   }
 
   auto DAddrSpace = GetGlobalVarAddressSpace(D);
-  auto TargetAddrSpace = getContext().getTargetAddressSpace(DAddrSpace);
 
   auto *GV = new llvm::GlobalVariable(
   getModule(), Ty, false, llvm::GlobalValue::ExternalLinkage, nullptr,
   MangledName, nullptr, llvm::GlobalVariable::NotThreadLocal,
-  TargetAddrSpace);
+  getContext().getTargetAddressSpace(DAddrSpace));
 
   // If we already created a global with the same mangled name (but different
   // type) before, take its name and remove it from its parent.
@@ -4005,10 +4006,10 @@
   LangAS ExpectedAS =
   D ? D->getType().getAddressSpace()
 : (LangOpts.OpenCL ? LangAS::opencl_global : LangAS::Default);
-  assert(getContext().getTargetAddressSpace(ExpectedAS) == AddrSpace);
+  assert(getContext().getTargetAddressSpace(ExpectedAS) == TargetAS);
   if (DAddrSpace != ExpectedAS) {
 return getTargetCodeGenInfo().performAddrSpaceCast(
-*this, GV, DAddrSpace, ExpectedAS, Ty->getPointerTo(AddrSpace));
+*this, GV, DAddrSpace, ExpectedAS, Ty->getPointerTo(TargetAS));
   }
 
   return GV;
@@ -4098,8 +4099,7 @@
 Ty = getTypes().ConvertTypeForMem(ASTTy);
 
   StringRef MangledName = getMangledName(D);
-  return GetOrCreateLLVMGlobal(MangledName, Ty,
-   getContext().getTargetAddressSpace(ASTTy), D,
+  return GetOrCreateLLVMGlobal(MangledName, Ty, ASTTy.getAddressSpace(), D,
   

[PATCH] D108449: [clang][NFC] Tighten up code for GetGlobalVarAddressSpace

[Andy Wingo via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8da70fed704c: [clang][NFC] Tighten up code for 
GetGlobalVarAddressSpace (authored by wingo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108449/new/

https://reviews.llvm.org/D108449

Files:
  clang/lib/CodeGen/CodeGenModule.cpp


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4148,16 +4148,15 @@
 }
 
 LangAS CodeGenModule::GetGlobalVarAddressSpace(const VarDecl *D) {
-  LangAS AddrSpace = LangAS::Default;
   if (LangOpts.OpenCL) {
-AddrSpace = D ? D->getType().getAddressSpace() : LangAS::opencl_global;
-assert(AddrSpace == LangAS::opencl_global ||
-   AddrSpace == LangAS::opencl_global_device ||
-   AddrSpace == LangAS::opencl_global_host ||
-   AddrSpace == LangAS::opencl_constant ||
-   AddrSpace == LangAS::opencl_local ||
-   AddrSpace >= LangAS::FirstTargetAddressSpace);
-return AddrSpace;
+LangAS AS = D ? D->getType().getAddressSpace() : LangAS::opencl_global;
+assert(AS == LangAS::opencl_global ||
+   AS == LangAS::opencl_global_device ||
+   AS == LangAS::opencl_global_host ||
+   AS == LangAS::opencl_constant ||
+   AS == LangAS::opencl_local ||
+   AS >= LangAS::FirstTargetAddressSpace);
+return AS;
   }
 
   if (LangOpts.SYCLIsDevice &&


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4148,16 +4148,15 @@
 }
 
 LangAS CodeGenModule::GetGlobalVarAddressSpace(const VarDecl *D) {
-  LangAS AddrSpace = LangAS::Default;
   if (LangOpts.OpenCL) {
-AddrSpace = D ? D->getType().getAddressSpace() : LangAS::opencl_global;
-assert(AddrSpace == LangAS::opencl_global ||
-   AddrSpace == LangAS::opencl_global_device ||
-   AddrSpace == LangAS::opencl_global_host ||
-   AddrSpace == LangAS::opencl_constant ||
-   AddrSpace == LangAS::opencl_local ||
-   AddrSpace >= LangAS::FirstTargetAddressSpace);
-return AddrSpace;
+LangAS AS = D ? D->getType().getAddressSpace() : LangAS::opencl_global;
+assert(AS == LangAS::opencl_global ||
+   AS == LangAS::opencl_global_device ||
+   AS == LangAS::opencl_global_host ||
+   AS == LangAS::opencl_constant ||
+   AS == LangAS::opencl_local ||
+   AS >= LangAS::FirstTargetAddressSpace);
+return AS;
   }
 
   if (LangOpts.SYCLIsDevice &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108450: [clang][CodeGen] GetDefaultAlignTempAlloca uses preferred alignment

[Andy Wingo via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4fb0c083429a: [clang][CodeGen] GetDefaultAlignTempAlloca 
uses preferred alignment (authored by wingo).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108450/new/

https://reviews.llvm.org/D108450

Files:
  clang/lib/CodeGen/CGExpr.cpp


Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -122,7 +122,7 @@
 Address CodeGenFunction::CreateDefaultAlignTempAlloca(llvm::Type *Ty,
   const Twine &Name) {
   CharUnits Align =
-CharUnits::fromQuantity(CGM.getDataLayout().getABITypeAlignment(Ty));
+  CharUnits::fromQuantity(CGM.getDataLayout().getPrefTypeAlignment(Ty));
   return CreateTempAlloca(Ty, Align, Name);
 }
 


Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -122,7 +122,7 @@
 Address CodeGenFunction::CreateDefaultAlignTempAlloca(llvm::Type *Ty,
   const Twine &Name) {
   CharUnits Align =
-CharUnits::fromQuantity(CGM.getDataLayout().getABITypeAlignment(Ty));
+  CharUnits::fromQuantity(CGM.getDataLayout().getPrefTypeAlignment(Ty));
   return CreateTempAlloca(Ty, Align, Name);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108464: [clang][CodeGen] Refactor CreateTempAlloca function nest. NFC.

[Andy Wingo via Phabricator via cfe-commits]

wingo updated this revision to Diff 368098.
wingo added a comment.

Rebase to no longer require Address default constructor.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108464/new/

https://reviews.llvm.org/D108464

Files:
  clang/lib/CodeGen/CGBuilder.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGCleanup.cpp
  clang/lib/CodeGen/CGCoroutine.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CGGPUBuiltin.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h

Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -576,7 +576,7 @@
 
   /// A mapping from NRVO variables to the flags used to indicate
   /// when the NRVO has been applied to this variable.
-  llvm::DenseMap NRVOFlags;
+  llvm::DenseMap NRVOFlags;
 
   EHScopeStack EHStack;
   llvm::SmallVector LifetimeExtendedCleanupStack;
@@ -623,11 +623,11 @@
 
   /// The exception slot.  All landing pads write the current exception pointer
   /// into this alloca.
-  llvm::Value *ExceptionSlot = nullptr;
+  Address ExceptionSlot = Address::invalid();
 
   /// The selector slot.  Under the MandatoryCleanup model, all landing pads
   /// write the current selector value into this alloca.
-  llvm::AllocaInst *EHSelectorSlot = nullptr;
+  Address EHSelectorSlot = Address::invalid();
 
   /// A stack of exception code slots. Entering an __except block pushes a slot
   /// on the stack and leaving pops one. The __exception_code() intrinsic loads
@@ -704,11 +704,11 @@
 
 /// An i1 variable indicating whether or not the @finally is
 /// running for an exception.
-llvm::AllocaInst *ForEHVar;
+Address ForEHVar = Address::invalid();
 
 /// An i8* variable into which the exception pointer to rethrow
 /// has been saved.
-llvm::AllocaInst *SavedExnVar;
+Address SavedExnVar = Address::invalid();
 
   public:
 void enter(CodeGenFunction &CGF, const Stmt *Finally,
@@ -2474,54 +2474,58 @@
 TBAAAccessInfo *TBAAInfo = nullptr);
   LValue EmitLoadOfPointerLValue(Address Ptr, const PointerType *PtrTy);
 
-  /// CreateTempAlloca - This creates an alloca and inserts it into the entry
-  /// block if \p ArraySize is nullptr, otherwise inserts it at the current
-  /// insertion point of the builder. The caller is responsible for setting an
-  /// appropriate alignment on
-  /// the alloca.
+  /// CreateTempAllocaInAS - Create an alloca in \p AddressSpace with alignment
+  /// \p Align.
   ///
   /// \p ArraySize is the number of array elements to be allocated if it
-  ///is not nullptr.
+  /// is not nullptr.
+  ///
+  /// The alloca will be inserted into the entry block if \p ArraySize is
+  /// nullptr.  Otherwise it is inserted at the current insertion point of the
+  /// builder.
+  Address CreateTempAllocaInAS(llvm::Type *Ty, CharUnits Align,
+   LangAS AddrSpace, const Twine &Name = "tmp",
+   llvm::Value *ArraySize = nullptr);
+
+  /// CreateTempAlloca - Create an alloca with CreateTempAllocaInAS, then cast
+  /// the result to LangAS::Default if necessary.
   ///
   /// LangAS::Default is the address space of pointers to local variables and
-  /// temporaries, as exposed in the source language. In certain
-  /// configurations, this is not the same as the alloca address space, and a
-  /// cast is needed to lift the pointer from the alloca AS into
-  /// LangAS::Default. This can happen when the target uses a restricted
-  /// address space for the stack but the source language requires
-  /// LangAS::Default to be a generic address space. The latter condition is
-  /// common for most programming languages; OpenCL is an exception in that
-  /// LangAS::Default is the private address space, which naturally maps
-  /// to the stack.
+  /// temporaries, as exposed in the source language. In certain configurations,
+  /// this is not the same as the alloca address space, and a cast is needed to
+  /// lift the pointer from the alloca AS into LangAS::Default. This can happen
+  /// when the target uses a restricted address space for the stack but the
+  /// source language requires LangAS::Default to be a generic address
+  /// space. The latter condition is common for most programming languages;
+  /// OpenCL is an exception in that LangAS::Default is the private address
+  /// space, which naturally maps to the stack.
   ///
   /// Because the address of a temporary is often exposed to the program in
   /// various ways, this function will perform the cast. The original alloca
   //

[PATCH] D108458: [clang][CodeGen] Add default constructor for Address. NFC.

[Andy Wingo via Phabricator via cfe-commits]

wingo abandoned this revision.
wingo added a comment.

In D108458#2957789 , @rjmccall wrote:

> You can still use a type without a default constructor in a `DenseMap`; you 
> just have to use `insert` instead of `dict[key] = ...`.

Aaaah, thanks for this note.  Closing this PR, then.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108458/new/

https://reviews.llvm.org/D108458

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108459: [clang][CodeGen] Rely on implicitly invalid Address. NFC.

[Andy Wingo via Phabricator via cfe-commits]

wingo abandoned this revision.
wingo added a comment.

Closing PR as it turns out that I don't need the Address() change for my work; 
no need to churn here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108459/new/

https://reviews.llvm.org/D108459

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108464: [clang][CodeGen] Refactor CreateTempAlloca function nest. NFC.

[Andy Wingo via Phabricator via cfe-commits]

wingo added a subscriber: sbc100.
wingo added a comment.

Thanks again John & Thomas for your thoughts.

In D108464#2961595 , @rjmccall wrote:

> In D108464#2960791 , @tlively wrote:
>
>> I don't think it makes sense to present these concepts as "address spaces" 
>> to C/C++ users, but that's what we're using at the IR level.
>
> Yeah, at some point I'm willing to accept that this is your best option at 
> the IR level, but I want to not jam this into the user-facing language unless 
> it's really the right design approach.

I am absolutely not married to the particular approach in this patch series and 
am happy to explore the design space :)

In D108464#2960791 , @tlively wrote:

> @wingo, are there cases where it is useful to declare variables as living in 
> WebAssembly locals and not in the VM stack? I'm having trouble coming up with 
> a case where leaving that up to the backend is not enough. We clearly need a 
> way to prevent values from being written to main memory (AS 0), but it's not 
> clear to me that we need a way to specifically allocate locals for them.

No, there are no cases that I know of.  From the IR & backend POV, the point as 
you say is to prevent values from being written to main memory.  It doesn't 
matter if they are in named locals or just temporaries.

The issue is that clang always lowers local variables as alloca's.  Clang needs 
to lower them to alloca's in AS 1 for reference types.  Without optimization, 
LLVM will lower an alloca in AS 1 to a WebAssembly local.  SSA conversion in 
SROA could lift it to an SSA variable which may avoid the local, in some cases. 
 So we are not specifically allocating a local for them, I agree that isn't the 
right way to express the requirement.

Also, being able to annotate non-reference-types with a `wasm_var` address 
space is not really part of the requirements.  It certainly has no utility for 
variables with automatic storage duration.  @sbc100 did mention that it could 
be useful for non-reference-typed globals, though, for ABI reasons.

In D108464#2961595 , @rjmccall wrote:

> Right now this seems to be entirely type-directed: it's a special property of 
> a couple of builtin types that you can't take the address of their objects 
> and those objects can only live in specific places.  Having it be 
> type-directed, but only to the point of saying that certain types *must* use 
> certain address spaces, and then imposing all the other restrictions on those 
> types as novel restrictions on those address spaces, feels like it's adding 
> complexity to the language concept of address spaces without much benefit.

Yeah I could be getting this wrong here.

To expand a bit, it's a special property of a class of builtin types -- 
currently `externref` and `funcref` but future WebAssembly specifications 
 will include reference types like 
`(struct i32 externref)`.  So there is a concept to factor out here that isn't 
specific to just `externref` and `funcref`.

@rjmccall do you think some kind of type attribute would make more sense?  If 
there is something already existing that I can take as a model, that would be 
helpful of course.

The utility we get from LangAS right now is (1) that it's a clear path to get 
alloca in AS 1 in codegen and (2) that it's a concept that Sema/ can query in 
order to detect errors and signal them to the user.  I am sure there are other 
ways to do this though!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108464/new/

https://reviews.llvm.org/D108464

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108360: [clang][NFC] Remove dead code

[Andy Wingo via Phabricator via cfe-commits]

wingo added a comment.

@Anastasia is this good to go for you or does this need closer attention?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108360/new/

https://reviews.llvm.org/D108360

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits