Stream: general

Topic: ask help for iadd_ifcout.


view this post on Zulip yang yu (Jul 11 2022 at 23:59):

https://docs.rs/cranelift-codegen/latest/cranelift_codegen/ir/trait.InstBuilder.html#method.iadd_ifcout

I thinking iadd_ifcout should be implment as "overflowing_add".
https://godbolt.org/z/7Mx5Mq4Y7
is this right?

my second question is how come c_out can be test with intcc.
c_out be a bool value only have two value,true and false.

pub fn add(a: i8, b: i8) -> (i8, bool) { a.overflowing_add(b) }

view this post on Zulip fitzgen (he/him) (Jul 12 2022 at 00:23):

the output a is the overflowed value and the output cout is whether overflow happened or not

view this post on Zulip yang yu (Jul 12 2022 at 00:35):

where is the intcc used??
do you mean in this case intcc can only be overflow or nof, no other intcc value is valid.

view this post on Zulip Afonso Bordado (Jul 13 2022 at 11:35):

my second question is how come c_out can be test with intcc.
c_out be a bool value only have two value,true and false.

We have 2 variations of this instruction iadd_cout and iadd_ifcout.

iadd_cout works exactly as you describe, by returning the integer and a bool representing whether overflowed has occured or not.

iadd_ifcout is slightly different in that it returns a cpuflags type. which must be tested with intcc or used in one of the flags branching instructions.

This second instruction relies on some mechanisms from cranelift that guarantee that the flags register won't be changed between this iadd_ifcout and the following brif / intcc / etc..

However given that RISCV doesn't have a integer flags register, I'm not sure we can implement this without for example pattern matching both a brif + iadd_ifcout or something similar. (And I'm not sure that is the best way to go about this.)

view this post on Zulip Afonso Bordado (Jul 13 2022 at 11:37):

There was some discussion about removing flags values a while ago. See:
https://github.com/bytecodealliance/wasmtime/issues/3249

In #3233 we discussed various simplifications we could perform to reduce the number of CLIF opcodes that exist. In general, we should strive for simplicity -- fewer opcodes mean less cost for every...

view this post on Zulip yang yu (Jul 14 2022 at 00:06):

ok,thanks.

view this post on Zulip yang yu (Aug 16 2022 at 02:17):

riscv64 can't lower iadd_ifcout right now , because riscv has no iflags register.
https://docs.rs/cranelift/latest/cranelift/prelude/types/constant.IFLAGS.html
I can lower where it's used like this.

(rule
  (lower (trapif _ (iadd_ifcout_parameters a b ty) trap_code ))
  (let
    (
      (tmp InstOutput (lower_iadd_overflow  a b ty  ) )
      (test Reg (inst_output_get tmp 1) )
    )
    (side_effect (SideEffectNoResult.Inst(MInst.TrapIf test trap_code )))
  )
)

but I can't sink the inst as it's lowered. I tried to call sink_inst but rightnow we can't sink a inst that have side effect.

view this post on Zulip Chris Fallin (Aug 16 2022 at 05:51):

@yang yu it might be helpful to look at what other backends do for iadd_ifcount. For example, in the x64 backend here, we return the add result, and an invalid_reg. This is because flags are not represented at the virtual-register level in VCode. Instead, instructions that consume flags have patterns that match the combination directly: for example, trapif using the flags result of iadd_ifcout has a pattern for (trapif (iadd_ifcout ...)), here for x64.

A standalone runtime for WebAssembly. Contribute to bytecodealliance/wasmtime development by creating an account on GitHub.
A standalone runtime for WebAssembly. Contribute to bytecodealliance/wasmtime development by creating an account on GitHub.

view this post on Zulip yang yu (Aug 17 2022 at 04:59):

@Chris Fallin but riscv have no iflags register , I think I still need compute overflow at trapif lowering point.
I can lowering iadd_ifcout just like other platforms.

view this post on Zulip Chris Fallin (Aug 17 2022 at 16:05):

yes, exactly, you'll need to compute overflow somehow; that's needed to implement the semantics of trap-on-overflow

view this post on Zulip Chris Fallin (Aug 17 2022 at 16:05):

probably a compare instruction would do it?

view this post on Zulip Jamey Sharp (Aug 17 2022 at 16:34):

according to the book "Hacker's Delight" by Henry S. Warren, Jr, I think not(x) < y (with unsigned comparison) is the simplest way.

view this post on Zulip Jamey Sharp (Aug 17 2022 at 16:37):

actually maybe it's better to reuse the result of the addition: x + y < x

view this post on Zulip yang yu (Aug 17 2022 at 23:47):

@Jamey Sharp it seems a little hard to reuse the result.

view this post on Zulip yang yu (Aug 25 2022 at 05:01):

@Chris Fallin is it true that a trap_if take an 'iadd_ifcout' as input , then intcc is not used?
and the add is a unsigned add??

view this post on Zulip Chris Fallin (Aug 25 2022 at 05:06):

@yang yu the flags are still used; the cout ("carry out") is a bit of an incorrect name, really it should just produce flags from the addition in general

view this post on Zulip Chris Fallin (Aug 25 2022 at 05:06):

see here where x64 pattern-matches on (trapif (iadd_ifcout ...)): https://github.com/bytecodealliance/wasmtime/blob/d3c463aac05e9ee3fd19450b3f035c47b9e55616/cranelift/codegen/src/isa/x64/lower.isle#L1420

A fast and secure runtime for WebAssembly. Contribute to bytecodealliance/wasmtime development by creating an account on GitHub.

view this post on Zulip Chris Fallin (Aug 25 2022 at 05:07):

in practice the opcode is used to test for overflow on addition, and this condition code, which is returned by an ISA-specific function, is used to test for overflow from the add: https://github.com/bytecodealliance/wasmtime/blob/d3c463aac05e9ee3fd19450b3f035c47b9e55616/cranelift/codegen/src/isa/mod.rs#L249

A fast and secure runtime for WebAssembly. Contribute to bytecodealliance/wasmtime development by creating an account on GitHub.

view this post on Zulip Chris Fallin (Aug 25 2022 at 05:08):

so to summarize: (trapif cc (iadd_ifcout a b)), where cc is the condition code returned by the trait method above, should trap if the iadd does an unsigned add overflow

view this post on Zulip Chris Fallin (Aug 25 2022 at 05:08):

hope that helps!

view this post on Zulip yang yu (Aug 25 2022 at 05:22):

I thinks I kind of understand it now.
image.png
this is from s390x backend , I think s390x and risc-v in the same situation.
can't use intcc to express overflow.

view this post on Zulip Chris Fallin (Aug 25 2022 at 06:09):

yup, exactly, s390x is the same here


Last updated: Nov 22 2024 at 16:03 UTC