Wasmtime GitHub notifications bot (Mar 04 2020 at 21:37): Wasmtime GitHub notifications bot (Mar 04 2020 at 21:37):iximeow submitted PR Review.
Wasmtime GitHub notifications bot (Mar 04 2020 at 21:37):iximeow submitted PR Review.
Wasmtime GitHub notifications bot (Mar 04 2020 at 21:37):iximeow created PR Review Comment:
I am a little :crying_cat_face: that the manual (at least the latest one I see online: https://software.intel.com/en-us/download/intel-64-and-ia-32-architectures-sdm-combined-volumes-1-2a-2b-2c-2d-3a-3b-3c-3d-and-4 from October 2019) doesn't seem to say anything about these bits being inverted in EVEX prefixes. In fact, just reading the manual suggests the opposite:
Combine with ModR/M.reg, ModR/M.rm (base, index/vidx), fromTable 2-30. EVEX Prefix Bit Field Functional Grouping.Anyway, I see the negated interpretation is backed up by XED so :+1:
Wasmtime GitHub notifications bot (Mar 04 2020 at 21:37):iximeow created PR Review Comment:
It's a bit defensive, but do you think a
debug_assert!(enc.mm() < 0b100)makes sense here?VEX.mmmmm > 0b10is also an error at the moment, and I suspect we'd need to look at encoding rules again if that changed too. So the most likely case I imagine forenc.mm() > 0b11is "Cranelift error". I also see thatmm()returns the bits masked by0x3anyway, so if this is in use when higher values become allocated (and we want to use them) there might need to be other places that need fixing as well.
Wasmtime GitHub notifications bot (Mar 04 2020 at 21:37):iximeow created PR Review Comment:
/// used together for certain classes of instructions; i.e., special care should be taken to ensureI think this is the missing word, anyway?
Wasmtime GitHub notifications bot (Mar 04 2020 at 22:21):abrown submitted PR Review.
Wasmtime GitHub notifications bot (Mar 04 2020 at 22:21):abrown created PR Review Comment:
Yeah, me too... mostly because I implemented it without inversion first and then had to invert it after getting weird results. I will ask around here to see if anyone knows what team is responsible for editing the manual.
Wasmtime GitHub notifications bot (Mar 05 2020 at 19:26):abrown updated PR #1192 from evex to master:
Wasmtime GitHub notifications bot (Mar 05 2020 at 19:32):abrown updated PR #1192 from evex to master:
Wasmtime GitHub notifications bot (Mar 05 2020 at 21:16):abrown updated PR #1192 from evex to master:
Wasmtime GitHub notifications bot (Mar 05 2020 at 21:17):abrown submitted PR Review.
Wasmtime GitHub notifications bot (Mar 05 2020 at 21:17):abrown created PR Review Comment:
I added it.
Wasmtime GitHub notifications bot (Mar 05 2020 at 21:18):abrown submitted PR Review.
Wasmtime GitHub notifications bot (Mar 05 2020 at 21:18):abrown created PR Review Comment:
Added this in a fixup and rebased so that the history looks cleaner; like we discussed, I think there are some bug fixes here that we won't want to squash together so I have been carefully rebasing.
Wasmtime GitHub notifications bot (Mar 05 2020 at 21:19):abrown submitted PR Review.
Wasmtime GitHub notifications bot (Mar 05 2020 at 21:19):abrown created PR Review Comment:
Ok, I e-mailed someone internally about this. Let's see what happens.
Wasmtime GitHub notifications bot (Mar 05 2020 at 21:34):iximeow submitted PR Review.
Wasmtime GitHub notifications bot (Mar 05 2020 at 21:39):yurydelendik submitted PR Review.
Wasmtime GitHub notifications bot (Mar 05 2020 at 21:39):yurydelendik submitted PR Review.
Wasmtime GitHub notifications bot (Mar 06 2020 at 18:53):abrown merged PR #1192.
Last updated: Dec 23 2024 at 13:07 UTC