This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] `AsmTypeCheck` support to br instr
ClosedPublic

Authored by HerrCai0907 on Apr 11 2023, 2:36 PM.

Details

Summary

This PR introduces the BrStack member to store the info about
loop, block, if and try. It can check whether br immediate number
out of range.

Diff Detail

Event Timeline

HerrCai0907 created this revision.Apr 11 2023, 2:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 11 2023, 2:36 PM
HerrCai0907 requested review of this revision.Apr 11 2023, 2:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 11 2023, 2:36 PM

Thanks for the simplified version!

llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp
144–146

I think it'd be better to start BrStack as the function return signature in the beginning than treating this as a special case. Currently we cannot check if the stack-remaining value types at the time of br matches the function return signature.

147
llvm/test/MC/WebAssembly/type-checker-errors.s
744

try~end_try without a catch/catch_all is malformed.

749–750
772

Can we add a few more tests on the depth?

  1. When the depth is +1 than the max possible depth (including the function boundary) (incorrect)
  2. When the depth targets the function boundary (correct)
  3. When the depth targets the function boundary, but the signature doesn't match (incorrect)

push func body in BrStack

HerrCai0907 marked 2 inline comments as done.

add more testcases

aheejin added inline comments.Apr 12 2023, 8:50 AM
llvm/test/MC/WebAssembly/type-checker-errors.s
744

Nit: I think tag_f32 (as in my previous suggestion) is better, because tag_i32 means introducing another type error within the catch. But we have another test for checking catch part below.

772

Ping. I don't think any of these were added?

HerrCai0907 marked 5 inline comments as done.Apr 12 2023, 11:49 AM

add testcase

aheejin accepted this revision.Apr 12 2023, 11:37 PM
This revision is now accepted and ready to land.Apr 12 2023, 11:37 PM
This revision was automatically updated to reflect the committed changes.