Add ISLE rule for heap offset symbol

[aborg-dev]

This removes the hacky implementation using LoadExtName in favor of a more targeted rule.

Note, that this is also an example where a rule for immediate addition would be useful and where the basic constant folding pass would not work. So #105 actually becomes quite appealing.

[mooori]

The TODO that was removed from emit.rs mentions having at most one external constant. Hardcoding 0 here means that still at most one external constant is expected?

[aborg-dev]

Not quite. The TODO was out of date, as we support 3 types of external constants now:

wasmtime/cranelift/codegen/src/isa/zkasm/lower/isle.rs

Line 373 in 7605474

fn zkasm_base(&mut self, global_value: GlobalValue) -> Option<generated_code::ZkasmBase> {

But these constants appear in slightly different contexts:

• For Heap, we use RegOffset addressing mode
• For Globals, we use Global addressing mode
• Table is currently not implemented
  I looked for a way to handle the Heap and Globals access similarly, but haven't found it because the generated CLIF code for them is slightly different.

[mooori]

Thank you for the context. I'm struggling to understand why there is a hardcoded 0 on the right hand side of this ISLE rule (instead of any other number)?

[mooori]

Perhaps I should add why I’m struggling: In the previous ISLE rule (that was replaced) name and offset parameters were passed on to load_ext_name. So to see what happens for the symbol_value lowering it was possible to look at how MInst::LoadExtName is handled.

With the new right hand side (imm $I32 0) for the ZkasmBase.Heap case, it’s not clear to me why the hardcoded value is 0 and how this 0 ends up in RegOffset.

[aborg-dev]

Ah, I see. We use 0 here as the memory region that we use on ZKASM machine starts at offset 0. The offset in global value is populated by us in zkasm.rs Environment

wasmtime/cranelift/wasm/src/environ/zkasm.rs

Line 267 in 7605474

offset: Imm64::new(0),

and is used to identify which memory region we want to access (Heap, Globals, Table). Does this make sense?

[mooori]

LGTM. I think a comment on the ISLE rule would be helpful. For example somehting like:

;; Having an `offset` of 0 in a `GlobalValue` signalizes it is stored on the heap, see
;; cranelift_wasm::environ::zkasm::ZkasmFuncEnvironment::heap_base

I’ve disabled auto-merge just for the case that you’d like to add a comment.

[aborg-dev]

LGTM. I think a comment on the ISLE rule would be helpful. For example somehting like:

;; Having an `offset` of 0 in a `GlobalValue` signalizes it is stored on the heap, see
;; cranelift_wasm::environ::zkasm::ZkasmFuncEnvironment::heap_base

I’ve disabled auto-merge just for the case that you’d like to add a comment.

Good idea, I've added the comments to clarify this.