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<WebAssemblyRegClass vt, Operand global_op> {
+multiclass LOCAL<WebAssemblyRegClass reg, Operand global_op> {
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
