Skip to content

[MIPS64] Failure to remove random NOPs in the middle of instructions #99783

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
GabrielRavier opened this issue Jul 20, 2024 · 2 comments · Fixed by #109386
Closed

[MIPS64] Failure to remove random NOPs in the middle of instructions #99783

GabrielRavier opened this issue Jul 20, 2024 · 2 comments · Fixed by #109386

Comments

@GabrielRavier
Copy link
Contributor

int f(int a, int b)
{
    return (a ^ b) | ~(a | b);
}

When compiled with -O3 -target mips64el -fomit-frame-pointer, LLVM outputs:

f(int, int): # @f(int, int)
  and $1, $5, $4
  sll $1, $1, 0
  not $1, $1
  jr $ra
  sll $2, $1, 0

whereas GCC manages (with -O3):

f(int, int):
  and $4,$4,$5
  jr $31
  nor $2,$0,$4

The sll instructions seem entirely unnecessary as they are just nops from what I understand, and they don't seem required for any particular reason (using -target mipsel instead of -target mips64el also gets them removed, so this seems quite specific to 64-bit MIPS)

@llvmbot
Copy link
Member

llvmbot commented Jul 20, 2024

@llvm/issue-subscribers-backend-mips

Author: Gabriel Ravier (GabrielRavier)

```cpp int f(int a, int b) { return (a ^ b) | ~(a | b); } ```

When compiled with -O3 -target mips64el -fomit-frame-pointer, LLVM outputs:

f(int, int): # @<!-- -->f(int, int)
  and $1, $5, $4
  sll $1, $1, 0
  not $1, $1
  jr $ra
  sll $2, $1, 0

whereas GCC manages (with -O3):

f(int, int):
  and $4,$4,$5
  jr $31
  nor $2,$0,$4

The sll instructions seem entirely unnecessary as they are just nops from what I understand, and they don't seem required for any particular reason (using -target mipsel instead of -target mips64el also gets them removed, so this seems quite specific to 64-bit MIPS)

@topperc
Copy link
Collaborator

topperc commented Jul 21, 2024

Based on the final SelectionDAG, it appears those sll instructions the truncate and the sign_extend

Optimized legalized selection DAG: %bb.0 'f:entry'                               
SelectionDAG has 16 nodes:                                                       
  t0: ch,glue = EntryToken                                                       
              t7: i64,ch = CopyFromReg t0, Register:i64 %1                       
            t8: i64 = AssertSext t7, ValueType:ch:i32                            
              t2: i64,ch = CopyFromReg t0, Register:i64 %0                       
            t4: i64 = AssertSext t2, ValueType:ch:i32                            
          t17: i64 = and t8, t4                                                  
        t18: i32 = truncate t17                                                  
      t12: i32 = xor t18, Constant:i32<-1>                                       
    t13: i64 = sign_extend t12                                                   
  t15: ch,glue = CopyToReg t0, Register:i64 $v0_64, t13                          
  t16: ch = MipsISD::Ret t15, Register:i64 $v0_64, t15:1

anbbna added a commit to anbbna/llvm-project that referenced this issue Sep 18, 2024
anbbna added a commit to anbbna/llvm-project that referenced this issue Sep 19, 2024
wzssyqa pushed a commit that referenced this issue Sep 19, 2024
Part of #99783 

This test is meant to reflect the oncoming change as this test shows the
unoptimized result with unnecessary SLLs.
anbbna added a commit to anbbna/llvm-project that referenced this issue Sep 20, 2024
anbbna added a commit to anbbna/llvm-project that referenced this issue Sep 20, 2024
anbbna added a commit to anbbna/llvm-project that referenced this issue Sep 20, 2024
anbbna added a commit to anbbna/llvm-project that referenced this issue Sep 30, 2024
anbbna added a commit to anbbna/llvm-project that referenced this issue Oct 9, 2024
anbbna added a commit to anbbna/llvm-project that referenced this issue Oct 25, 2024
anbbna added a commit to anbbna/llvm-project that referenced this issue Oct 29, 2024
Optimize '$dst = sign_extend (xor (trunc $src), imm)'
to '$dst = sign_extend (trunc (xor $src, imm))'.

Fix llvm#99783
anbbna added a commit to anbbna/llvm-project that referenced this issue Oct 29, 2024
Optimize '$dst = sign_extend (xor (trunc $src), imm)'
to '$dst = sign_extend (trunc (xor $src, imm))'.

Fix llvm#99783
Cyanoxygen pushed a commit to Cyanoxygen/llvm-project that referenced this issue Jan 26, 2025
Optimize '$dst = sign_extend (xor (trunc $src), imm)'
to '$dst = sign_extend (trunc (xor $src, imm))'.

Fix llvm#99783
Cyanoxygen pushed a commit to Cyanoxygen/llvm-project that referenced this issue Jan 26, 2025
Optimize '$dst = sign_extend (xor (trunc $src), imm)'
to '$dst = sign_extend (trunc (xor $src, imm))'.

Fix llvm#99783
anbbna added a commit to anbbna/llvm-project that referenced this issue Jan 26, 2025
The constant operand of the XOR is an i32, which makes the two operands
being truncated to i32, and sign extended to i64 afterwards. Since both
of the i32 operands are sign extended to i64 before performing the
TRUNCATE operation, we just remove the unnecessary TRUNCATE and
SIGN_EXTEND.

Fix llvm#99783
anbbna added a commit to anbbna/llvm-project that referenced this issue Feb 18, 2025
… X, i32), imm)

The constant operand of the XOR is an i32, which makes the two operands
being truncated to i32, and sign extended to i64 afterwards. Since both
of the i32 operands are sign extended to i64 before performing the
TRUNCATE operation, we just remove the unnecessary TRUNCATE and
SIGN_EXTEND.

Fix llvm#99783
anbbna added a commit to anbbna/llvm-project that referenced this issue May 7, 2025
…inreg X, i32), imm)

The constant operand of the XOR is an i32, which makes the two operands
being truncated to i32, and sign extended to i64 afterwards. Since both
of the i32 operands are sign extended to i64 before performing the
TRUNCATE operation, we just remove the unnecessary TRUNCATE and
SIGN_EXTEND.

Fix llvm#99783
anbbna added a commit to anbbna/llvm-project that referenced this issue May 7, 2025
…inreg X, i32), imm)

The constant operand of the XOR is an i32, which makes the two operands
being truncated to i32, and sign extended to i64 afterwards. Since both
of the i32 operands are sign extended to i64 before performing the
TRUNCATE operation, we just remove the unnecessary TRUNCATE and
SIGN_EXTEND.

Fix llvm#99783
@brad0 brad0 closed this as completed in 69f3552 May 7, 2025
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this issue May 7, 2025
…9386)

Optimize ((signext (xor (trunc X), imm)) to (xor (X, imm)).

Fix llvm/llvm-project#99783
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this issue May 7, 2025
Optimize ((signext (xor (trunc X), imm)) to (xor (X, imm)).

Fix llvm#99783
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants