evmar / retrowin32

windows emulator
https://evmar.github.io/retrowin32/
Apache License 2.0
587 stars 26 forks source link

Set flags in imul #57

Closed evmar closed 1 month ago

evmar commented 2 months ago

There are 4 TODOs here in imul, that we need to set flags: https://github.com/evmar/retrowin32/blob/b998e571ba9e31b4cc95e18f091376cb8372a9fb/x86/src/ops/math.rs#L787-L814

https://www.felixcloutier.com/x86/imul says:

The CF and OF flags are set when the signed integer value of the intermediate product differs from the sign extended operand-size-truncated product, otherwise the CF and OF flags are cleared.

The three forms of the IMUL instruction are similar in that the length of the product is calculated to twice the length of the operands. With the one-operand form, the product is stored exactly in the destination. With the two- and three- operand forms, however, the result is truncated to the length of the destination before it is stored in the destination register. Because of this truncation, the CF or OF flag should be tested to ensure that no significant bits are lost.

I think what this means is that CF/OF should just be cleared in the first three of those impls (because there is no truncation). imul_trunc should instead ... do something smart once I understand the first sentence of that quote.

evmar commented 2 months ago

I think what's saying is that for a like 16-bit mul, you multiply into 32 bits, then compare if truncating it matters.

let x = ... as i16;
let y = ... as i16;
let widened = (x as i32).wrapping_mul(y as i32);
let result = widened as i16;
let set_flags = widened != result;
LinusU commented 1 month ago

I've been experimenting with getting code that complies to the imul instruction, and I think it's really cool that this does!

Screenshot 2024-10-04 at 18 37 59

[Godbolt link](https://godbolt.org/#g:!((g:!((g:!((h:codeEditor,i:(filename:'1',fontScale:14,fontUsePx:'0',j:1,lang:rust,selection:(endColumn:1,endLineNumber:19,positionColumn:1,positionLineNumber:19,selectionStartColumn:1,selectionStartLineNumber:19,startColumn:1,startLineNumber:19),source:'fn+imul(a:+i32,+b:+i32)+-%3E+(i32,+bool)+%7B%0A++++let+wide+%3D+(a+as+i64)+*+(b+as+i64)%3B%0A++++let+result+%3D+wide+as+i32%3B%0A++++let+overflow+%3D+a.checked_mul(b).is_none()%3B%0A%0A++++(result,+overflow)%0A%7D%0A%0A%23%5Bno_mangle%5D%0Apub+fn+test(a:+i32,+b:+i32)+-%3E+u8+%7B%0A++++let+(_,+o)+%3D+imul(a,+b)%3B%0A%0A++++if+o+%7B%0A++++++++23%0A++++%7D+else+%7B%0A++++++++45%0A++++%7D%0A%7D%0A'),l:'5',n:'1',o:'Rust+source+%231',t:'0')),k:50,l:'4',n:'0',o:'',s:0,t:'0'),(g:!((g:!((h:compiler,i:(compiler:r1810,filters:(b:'0',binary:'1',binaryObject:'1',commentOnly:'0',debugCalls:'1',demangle:'0',directives:'0',execute:'1',intel:'0',libraryCode:'0',trim:'1',verboseDemangling:'0'),flagsViewOpen:'1',fontScale:14,fontUsePx:'0',j:1,lang:rust,libs:!(),options:'-C++++++++++++++++opt-level%3D3',overrides:!(),selection:(endColumn:1,endLineNumber:1,positionColumn:1,positionLineNumber:1,selectionStartColumn:1,selectionStartLineNumber:1,startColumn:1,startLineNumber:1),source:1),l:'5',n:'0',o:'+rustc+1.81.0+(Editor+%231)',t:'0')),k:50,l:'4',m:50,n:'0',o:'',s:0,t:'0'),(g:!((h:output,i:(compilerName:'x86-64+gcc+14.2',editorid:1,fontScale:14,fontUsePx:'0',j:1,wrap:'1'),l:'5',n:'0',o:'Output+of+rustc+1.81.0+(Compiler+%231)',t:'0')),header:(),l:'4',m:50,n:'0',o:'',s:0,t:'0')),k:50,l:'3',n:'0',o:'',t:'0')),l:'2',n:'0',o:'',t:'0')),version:4)


With that, I've managed to land in the following:

fn imul_trunc<I: SignedInt>(x: I, y: I, flags: &mut Flags) -> I {
    let wide = x.widen() * y.widen();
    let flag = x.checked_mul(&y).is_none();

    flags.set(Flags::OF, flag);
    flags.set(Flags::CF, flag);

    I::from_wide(wide)
}

Which still compiles to just calling imul 🤩 Godbolt link