microsoft / llvm-mctoll

llvm-mctoll
Other
816 stars 125 forks source link

[X86-64] Add support for SSE and, or and xor instructions #105

Closed martin-fink closed 3 years ago

martin-fink commented 3 years ago

This commit adds support for the following instructions:

This commit also adds support for vector return types and arguments, as the result from the above instructions are vectors and use the full 128 bits of the SSE registers.

SSE register values are now handled in the following way:

  1. If the reaching value to an instruction is smaller than expected, it is padded with 0's If it is smaller, the value is truncated and we just use the bytes we're interested in.
  2. The resulting value is then bitcast to the expected value (e.g. double for ADDSDrr).

Instructions like ADDSDrr write the result of the operation to the lower 64 bit of the register, while the upper 64 bits are zeroed. Since missing bits are assumed to be zero in step 1 anyway, we don't need to worry about the upper 64 bits and set the register value to double.

If an instruction also works on the upper 64 bits (e.g. PANDrr), we set the register value to the type of the result, which will be a vector (e.g. <4 x i32>) or another type that includes all 128 bits.

If an instruction only modifies lower bits, while preserving upper bits, we need to make sure to preserve those bits as well, but no instruction that does that is currently implemented in mctoll.

bharadwajy commented 3 years ago

Thanks @martin-fink for the PR.

The description in the commit message is much also appreciated. In addition, can you please add similar comments in the code that you added at appropriate places including e.g., adding a description on top of reinterpretSSEValue()?

Next, I see 4 regressions with the changes in this PR. Following are the details:

3 regressions (smoke_test/double_args.c, smoke_test/double_float_int_args.c, smoke_test/float_args.c) result from an attempt to cast double %arg2 (RetValue) to <4 x float> (RetType) while raising RETQ - which inturn results in assertion failure

<llvm-project-repo>/llvm/lib/IR/Instructions.cpp:3321: static Instruction::CastOps llvm::CastInst::getCastOpcode(const llvm::Value *, bool, llvm::Type *, bool): Assertion `DestBits == SrcBits && "Illegal cast to vector (wrong type or size)"'

at X86/X86MachineinstructionRaiser.cpp:4004

// Ensure RetValue type match RetType
  if (RetValue != nullptr)
    RetValue = getRaisedValues()->castValue(RetValue, RetType, RaisedBB);

Following is the basic-block information for each of the failures containing the RETQ instruction resulting in the failure.

smoke_test/double_float_int_args.c

# Machine code for function get_third: TracksLiveness

bb.0:
  $xmm0 = MOVAPSrr $xmm1, <0x7d45478>
  RETQ <0x7d45598>

# End machine code for function get_third.

smoke_test/double_args.c

# Machine code for function get_second: TracksLiveness

bb.0:
  $xmm0 = MOVAPSrr $xmm1, <0x7d8f378>
  RETQ <0x7d8f3f8>

# End machine code for function get_second.

smoke_test/float_args.c

# Machine code for function get_second: TracksLiveness

bb.0:
  $xmm0 = MOVAPSrr $xmm1, <0x7d8f4d8>
  RETQ <0x7d8f558>

# End machine code for function get_second.

1 regression asm_test/X86/raise-movq.c

<llvm-project-repo>/llvm/tools/llvm-mctoll/test/asm_test/X86/raise-movq.c:6:11: error: CHECK: expected string not found in input
// CHECK: 0x4045400000000000, 0x422a0000
          ^
<stdin>:1:1: note: scanning from here
0x4045400000000000, 0x00000000
^

Input file: <stdin>
Check file: <llvm-project-repo>/llvm/tools/llvm-mctoll/test/asm_test/X86/raise-movq.c

-dump-input=help explains the following input dump.

Input was:
<<<<<<
         1: 0x4045400000000000, 0x00000000 
check:6     X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
         2: 42.5, 42.5 
check:6     ~~~~~~~~~~~
>>>>>>
martin-fink commented 3 years ago

Thanks! I have now fixed the regressions and also modified the way stack promotions are done for SSE register values. SSE register values (vectors, floats, or doubles) are stored in a 128-bit integer stack slot. Values not large enough (e.g. float) are padded with 0's, as the missing bits are assumed to be 0.

BTW, I've kept the changes in two commits, since I thought that might make it easier to review. I'll squash them before merging though.

bharadwajy commented 3 years ago

Thanks @martin-fink. Pulled to master.