Skip to content
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

move not using branch delay slot #290

Closed
jonwoodruff opened this issue Sep 28, 2017 · 7 comments
Closed

move not using branch delay slot #290

jonwoodruff opened this issue Sep 28, 2017 · 7 comments

Comments

@jonwoodruff
Copy link

jonwoodruff commented Sep 28, 2017

int * __capability test(int * __capability inCap, int test, int inInt)
{
  int * __capability ret = inCap;
  if (test) {
    ret+=inInt+4;
  }
  return ret;
}

Function starts with:

	move	 $fp, $sp
	beqz	$4, .LBB0_2
	nop

But should produce:

	beqz	$4, .LBB0_2
	move	 $fp, $sp
@davidchisnall
Copy link
Member

Was this fixed by the big bunch of changes that stopped treating most CHERI instructions as having unmodeled side effects?

@jonwoodruff
Copy link
Author

So things have moved along, but right now we're still not using the branch delay slot. The assembly we're generating now is:

cincoffset	$c24, $c11, $zero
clc	$c3, $zero, 0($c3)
beqz	$4, .LBB0_2
nop

And it looks like either the cincoffset or the clc could be moved into the delay slot here.

@arichardson arichardson transferred this issue from CTSRD-CHERI/llvm Feb 18, 2019
@arichardson
Copy link
Member

This is happening due to CFI_INSTRUCTION pseudo opcodes terminating the DelaySlotFiller search.
I think it is not safe to move anything around past those instructions since it will break debug info and exceptions.

However, it seems like those are emitted even if we have exceptions disabled...

@jrtc27
Copy link
Member

jrtc27 commented Feb 18, 2019

Using an up-to-date upstream Clang to compile this on plain mips64 doesn't do the delay slot filling either at -O2:

test:
        daddiu  $sp, $sp, -16
        sd      $fp, 8($sp)
        move    $fp, $sp
        beqz    $5, .LBB0_2
        nop

GCC however (with -fno-omit-frame-pointer) will fill the delay slot:

        daddiu  $sp,$sp,-16
        sd      $fp,8($sp)
        beq     $5,$0,.L3
        move    $fp,$sp

This turns out to be ok for GDB; it has a list of instructions it allows to be in the prologue, and that includes instructions with delay slots. It scans forward until the end of the prologue (see the various *mips*_scan_prologue functions), and has the comment:

  /* Permit at most one non-prologue non-control-transfer instruction
     in the middle which may have been reordered by the compiler for
     optimisation.  */

So, I guess the fix is to teach LLVM that it can fill the first delay slot in the function with a prologue instruction, provided there are no non-prologue instructions in between. Having said that, our MIPS baseline using LLVM suffers from the same missing optimisation, so we still have a fair comparison.

@jrtc27
Copy link
Member

jrtc27 commented Dec 10, 2019

That commit doesn't really prove the issue has been fixed in general, just that we don't trip over it any more for this specific case due to using a conditional move rather than a branch. If inCap is made an address (or normal pointer in hybrid mode) then we can't use a conditional move and still see the unfilled delay slot.

@jrtc27 jrtc27 reopened this Dec 10, 2019
@arichardson
Copy link
Member

Since MIPS is also affected, I don't think we need to fix this here: https://cheri-compiler-explorer.cl.cam.ac.uk/z/6Dzy6U

We should probably report it upstream instead.

@arichardson
Copy link
Member

Closing issues that only affect MIPS.

@arichardson arichardson closed this as not planned Won't fix, can't repro, duplicate, stale Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants