Bug or feature? (`freeze` instruction)

LLVM Project
#1

Hi, we’re having issues after migrating from LLVM 10 to LLVM 11. We have the feeling this is a bug in LLVM, but perhaps we’re missing something. The problem has two parts to it, but they’re somewhat related so I’m posting them together.

Part 1, mysterious freeze instructions

The first part is related to freeze instructions that appears in our generated .ll files. We aren’t inserting them, so this is done by the C api. Why is that, and how can we disable it? I have a mild understanding of what this instruction does, but I don’t know the reasons to be automatically inserted into our code. In principle, they shouldn’t harm, however…

Part 2, incorrect? asm generation

There seems to be a bug in the generation of x86 asm code when there is a freeze operation. The bug only appears when performing div/rem operations on constants. The following two functions only differ in that in the first one the freeze operation is called before doing the urem operation on a truncated argument.

define fastcc i32 @frozen(i64 %a) nounwind {
entry:
  %x = trunc i64 %a to i32
  %y = freeze i32 %x
  %z = urem i32 %y, 10
  ret i32 %z
}
 
define fastcc i32 @thawn(i64 %a) nounwind {
entry:
  %x = trunc i64 %a to i32
  %z = urem i32 %x, 10
  ret i32 %z
}

However, the assembly code differs in a critical aspect: the code for the first one seems to “forget” to truncate the argument:

    movq    %rdi, %rax
    movl    $3435973837, %ecx               # imm = 0xCCCCCCCD
    imulq    %rdi, %rcx
    ...

Instead, the function without the freeze is compiled correctly:

    movq    %rdi, %rax
    movl    %eax, %ecx                      # <---- truncation here
    movl    $3435973837, %edx               # imm = 0xCCCCCCCD
    imulq    %rcx, %rdx
    ...

This only happens when the operation is performed on a constant, which lead us to believe it has to do with the optimization when doing division/remainder with a constant. The bug is not present if we disable optimizations.

Thanks a lot for any pointer to solve this issue! And please let us know if we should file it in the issue tracker.

#2

This should fix it if you can rebuild llvm.

diff --git a/llvm/lib/Target/X86/X86InstrCompiler.td b/llvm/lib/Target/X86/X86InstrCompiler.td
index d1e59bf..ffb66f2 100644
--- a/llvm/lib/Target/X86/X86InstrCompiler.td
+++ b/llvm/lib/Target/X86/X86InstrCompiler.td
@@ -1360,7 +1360,8 @@ def def32 : PatLeaf<(i32 GR32:$src), [{
          N->getOpcode() != TargetOpcode::EXTRACT_SUBREG &&
          N->getOpcode() != ISD::CopyFromReg &&
          N->getOpcode() != ISD::AssertSext &&
-         N->getOpcode() != ISD::AssertZext;
+         N->getOpcode() != ISD::AssertZext &&
+         N->getOpcode() != ISD::FREEZE;
 }]>;
1 Like
#3

Thanks @topperc ! If I can take a bit more of your time, do you mind sharing what’s going on? In particular, I’d like to know if this is indeed a bug worth being filed, and if a solution will land in following releases.

This said, I’d try your alternative, although it’s not a definitive solution for us as we can’t require our users to rebuild llvm.

#4

I can’t say for sure where the freeze came from. I know the DivRemPairs pass can insert a freeze. Are you able to determine when the freeze was inserted?

Because the division was by a constant, it was replaced with a sequence using multiply as described here Division algorithm - Wikipedia. This sequence needs a multiply that returns the high 32 bits of the full double width product. That gets replaced by a zext from i32 to i64, an i64 multiply, a shift by 32, and a truncate.

The code in my patch is part of a check that determines if an instruction needs to be emitted for the zext. Most 32-bit instructions on X86-64 zero the upper 32 bits of the destination register. The list of opcodes there are supposed to be the opcodes that won’t directly emit an instruction that zeros the upper bits. We failed to add freeze to that list since it isn’t guaranteed to emit an instruction. I believe it emits a copy that might be removed later.

A fix has been pushed to llvm trunk. I’ve requested a merge to 12.0.1, but the deadline for inclusion was June 11 so we’ll see what happens. 50695 – Merge c997867dc084a1bcf631816f964b3ff49a297ba3 to 12.0.1

1 Like
#5

Thanks a lot @topperc ! We also believe the freeze comes from the optimization DivRemPairs.