[llvm-commits] mips commit regarding RA register problem

[Kotler, Reed]

I have the whole rest of the mips 16 port sitting in our local repository.

I can't check it in all at once because it's too big.

So I need to check in all the pieces and then I can do more  work. This is what I'm trying to do right now.

What part of this check in is bothering you?

This code will work forever. It's correct.

What you complained about is the test for a 0 stack size, which  cannot happen right now because
I am forcing it to have a non zero stack size.

I don't want to delete that code because it will become allowable once again when I do the optimization which makes it possible to not save RA at all.

We CANNOT revert this patch right now and continue our work.

The code we are checking in works. Why is it a horrible hack? It is correct. it will work until the Sun runs out of fuel and stops heating the planet.

You want us to delete two lines of code that test that stack size is 0?

What problem can this cause?

There are NO bugs caused by this checkin.

The horrible bug is what happens without this checkin.

Reed


_______________________________________
From: Bill Wendling [wendling at apple.com]
Sent: Monday, September 17, 2012 4:37 PM
To: Kotler, Reed
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: [llvm-commits] mips commit regarding RA register problem

On Sep 17, 2012, at 3:40 PM, "Kotler, Reed" <rkotler at mips.com> wrote:

> This patch I submitted is needed to fix RA is temporary in a relative sense, it will be there for a few weeks until I have time to add the code so that saving RA is optional for Mips.
>
> When I make that patch, then the following line which checks for the stack size being 0 will be relevant again.
>
> This was a matter of optimizations and changes occurring in mips32 and mips64 that are not possible in Mips 16 at this time.
>
> The patch is needed or else many tests will fail.
>
> I'm in the process of trying to pass all the tests in test-suite and this bug was found.
>
> Nobody is using the mips 16 port now because it is not all checked in even.

This sounds like something that's affecting you locally. Why not have the hack in your checked out version of the compiler instead of putting it into the main trunk?

> I'm in the process of checking in the code.

Good. However, that doesn't address the important point here. We try *very* hard to keep the trunk as stable and bug-free as possible. This implies that hacks to work around problems are highly discouraged. Not only is it bad programming, but they might be missed and stay in the code base. Or worse, someone could come along and write code that depends upon the hack. It matters not whether this applies to this situation.

I *really* would prefer you revert the patch and keep the hack local to your code until you're ready to check in the correct code. If you refuse, then you need to at least comment that awful hack to make sure that no one relies upon it.

-bw