xmm in regset parsed as ymm about stoke HOT 12 CLOSED

ohh... now that I try to write a fix I see why this is. It looks like there's a typing issue (i.e. "type system" not "hitting the keyboard") because Xmm and Ymm don't inheirit from a common base class other than Operand. So, it looks like some plumming needs to be done. I could probably whip something up, but do you have a preference for how to resolve this?

from stoke.

This is a "feature that hides a bug", though off the top of my head, I
can't remember exactly what the bug was. There's a similar behavior with
general purpose registers. If you specify a 32-bit register as def-in, it
gets promoted to it's 64-bit parent. %ax becomes %rax, for instance.

I think that it had something to do with our dataflow analyses having
trouble with boundary conditions that weren't full registers. Making the
entire register def-in hid an undefined value read that we weren't able to
get rid of otherwise.

Even with that said, though. I think we ended up going into to the sandbox
code and copying the entire contents of a machine state into at least the
sse registers because we might STILL have been having problems.

So. If this is a crucial improvement for you, the strategy would be this:

1. Change the code in src/args/regset.[cc,h] to stop promoting registers
2. Go into the sandbox and change things so that the entire machine state,
   sse and gp, is copied to the cpu, regardless of what's def-in. This could
   turn out to be a performance hit, but like I said, we're already doing it
   with sse, so it shouldn't be too big.

As for the other bit --

I don't think it makes a difference that Xmm and Ymm don't share a common
base class (like Sse or something). The general purpose registers don't
either. From the assemblers point of view, this is a distinction (gp vs
sse) that never needs to be enforced. But if there's a reason I'm not
seeing, just add two more classes into the hierarchy (Gp and Sse) and it
should be fine. I don't see it affecting anything else in the x64asm
codebase.

On Wed, Oct 1, 2014 at 1:02 AM, Berkeley Churchill <[email protected]

wrote:

ohh... now that I try to write a fix I see why this is. It looks like
there's a typing issue (i.e. "type system" not "hitting the keyboard")
because Xmm and Ymm don't inheirit from a common base class other than
Operand. So, it looks like some plumming needs to be done. I could probably
whip something up, but do you have a preference for how to resolve this?

—
Reply to this email directly or view it on GitHub
#36 (comment).

from stoke.

Makes sense. Right now I'm working on making the analysis rock-solid in
my branch, and that's how I came across this. So this is more related
to what I care about than I thought. In my branch I'll make the changes
to parsing, and once we're all fixed up, hopefully we won't need to move
everything into the sandbox. Maybe at that point I'll be able to put
some changes back into develop. (I'm even thinking that it might make
sense to merge my whole branch into develop at some point, since I've
nicely organized all my work into a stoke_database tool and a database/
folder, and everything else are bug fixes/minor improvements).

On 10/01/2014 10:01 AM, eric schkufza wrote:

This is a "feature that hides a bug", though off the top of my head, I
can't remember exactly what the bug was. There's a similar behavior with
general purpose registers. If you specify a 32-bit register as def-in, it
gets promoted to it's 64-bit parent. %ax becomes %rax, for instance.

I think that it had something to do with our dataflow analyses having
trouble with boundary conditions that weren't full registers. Making the
entire register def-in hid an undefined value read that we weren't able to
get rid of otherwise.

Even with that said, though. I think we ended up going into to the sandbox
code and copying the entire contents of a machine state into at least the
sse registers because we might STILL have been having problems.

So. If this is a crucial improvement for you, the strategy would be this:

1. Change the code in src/args/regset.[cc,h] to stop promoting registers
2. Go into the sandbox and change things so that the entire machine state,
   sse and gp, is copied to the cpu, regardless of what's def-in. This could
   turn out to be a performance hit, but like I said, we're already doing it
   with sse, so it shouldn't be too big.

As for the other bit --

I don't think it makes a difference that Xmm and Ymm don't share a common
base class (like Sse or something). The general purpose registers don't
either. From the assemblers point of view, this is a distinction (gp vs
sse) that never needs to be enforced. But if there's a reason I'm not
seeing, just add two more classes into the hierarchy (Gp and Sse) and it
should be fine. I don't see it affecting anything else in the x64asm
codebase.

On Wed, Oct 1, 2014 at 1:02 AM, Berkeley Churchill <[email protected]

wrote:

ohh... now that I try to write a fix I see why this is. It looks like
there's a typing issue (i.e. "type system" not "hitting the keyboard")
because Xmm and Ymm don't inheirit from a common base class other than
Operand. So, it looks like some plumming needs to be done. I could probably
whip something up, but do you have a preference for how to resolve this?

—
Reply to this email directly or view it on GitHub
#36 (comment).

---

Reply to this email directly or view it on GitHub:
#36 (comment)

from stoke.

Sounds lovely. Just as a point of repository organization, try to keep the
scope of the branches sort of narrow. So one merge for database, another
for dataflow fixing, etc.
On Oct 1, 2014 3:37 PM, "Berkeley Churchill" [email protected]
wrote:

Makes sense. Right now I'm working on making the analysis rock-solid in
my branch, and that's how I came across this. So this is more related
to what I care about than I thought. In my branch I'll make the changes
to parsing, and once we're all fixed up, hopefully we won't need to move
everything into the sandbox. Maybe at that point I'll be able to put
some changes back into develop. (I'm even thinking that it might make
sense to merge my whole branch into develop at some point, since I've
nicely organized all my work into a stoke_database tool and a database/
folder, and everything else are bug fixes/minor improvements).

On 10/01/2014 10:01 AM, eric schkufza wrote:

This is a "feature that hides a bug", though off the top of my head, I
can't remember exactly what the bug was. There's a similar behavior with
general purpose registers. If you specify a 32-bit register as def-in, it
gets promoted to it's 64-bit parent. %ax becomes %rax, for instance.

I think that it had something to do with our dataflow analyses having
trouble with boundary conditions that weren't full registers. Making the
entire register def-in hid an undefined value read that we weren't able
to
get rid of otherwise.

Even with that said, though. I think we ended up going into to the
sandbox
code and copying the entire contents of a machine state into at least the
sse registers because we might STILL have been having problems.

So. If this is a crucial improvement for you, the strategy would be this:

1. Change the code in src/args/regset.[cc,h] to stop promoting registers
2. Go into the sandbox and change things so that the entire machine
   state,
   sse and gp, is copied to the cpu, regardless of what's def-in. This could
   turn out to be a performance hit, but like I said, we're already doing it
   with sse, so it shouldn't be too big.

As for the other bit --

I don't think it makes a difference that Xmm and Ymm don't share a common
base class (like Sse or something). The general purpose registers don't
either. From the assemblers point of view, this is a distinction (gp vs
sse) that never needs to be enforced. But if there's a reason I'm not
seeing, just add two more classes into the hierarchy (Gp and Sse) and it
should be fine. I don't see it affecting anything else in the x64asm
codebase.

On Wed, Oct 1, 2014 at 1:02 AM, Berkeley Churchill <
[email protected]

wrote:

ohh... now that I try to write a fix I see why this is. It looks like
there's a typing issue (i.e. "type system" not "hitting the keyboard")
because Xmm and Ymm don't inheirit from a common base class other than
Operand. So, it looks like some plumming needs to be done. I could
probably
whip something up, but do you have a preference for how to resolve this?

—
Reply to this email directly or view it on GitHub
#36 (comment).

---

Reply to this email directly or view it on GitHub:
#36 (comment)

—
Reply to this email directly or view it on GitHub
#36 (comment).

from stoke.

This is, by the way, why all the tests currently failing in 'develop' are failing. I have a fix ready, I just don't want to break your underlying bug with the merge.

from stoke.

Circling back to this --

Now that we have a testing infrastructure, checking for non-determinism in cost function evaluations would be huge. The behavior you want to simulate is:

1. evaluate cost. make a change. evaluate cost. undo the change. evaluate cost. Check that the costs are the same.
2. evaluate cost. evaluate cost. Check to make sure the costs are the same.

If we have this, we can go ahead of try pushing this change to see if it's still hiding a bug or not.

from stoke.

O, awesome. I'll look into writing these tests when I get a chance.

On Fri, Oct 10, 2014 at 3:41 PM, eric schkufza [email protected]
wrote:

Circling back to this --

Now that we have a testing infrastructure, checking for non-determinism in
cost function evaluations would be huge. The behavior you want to simulate
is:

evaluate cost. make a change. evaluate cost. undo the change. evaluate
cost. Check that the costs are the same.
2.

evaluate cost. evaluate cost. Check to make sure the costs are the
same.

If we have this, we can go ahead of try pushing this change to see if it's
still hiding a bug or not.

—
Reply to this email directly or view it on GitHub
#36 (comment).

from stoke.

The tests are here, and none failed, although I can't be certain of how thorough they are. I'm not sure how to copy all of the defined registers, I only looked briefly but I don't see where that is in the code. I think I'll merge in the fix I've been using on my branches for the last few weeks, and if nothing breaks, we can call it good for now.

from stoke.

Sounds good.

Can you also go through the network graph and delete the branches that
you've merged back into develop?

On Mon, Oct 13, 2014 at 1:02 PM, Berkeley Churchill <
[email protected]> wrote:

The tests are here, and none failed, although I can't be certain of how
thorough they are. I'm not sure how to copy all of the defined registers, I
only looked briefly but I don't see where that is in the code. I think I'll
merge in the fix I've been using on my branches for the last few weeks, and
if nothing breaks, we can call it good for now.

—
Reply to this email directly or view it on GitHub
#36 (comment).

from stoke.

I've been leaving some around just in case we need to go back for some
reason. But, I've been deleting ones that are over 7 days old that have
been merged in. Does that sound alright?

Berkeley

On 10/13/2014 01:03 PM, eric schkufza wrote:

Sounds good.

Can you also go through the network graph and delete the branches that
you've merged back into develop?

On Mon, Oct 13, 2014 at 1:02 PM, Berkeley Churchill <
[email protected]> wrote:

The tests are here, and none failed, although I can't be certain of how
thorough they are. I'm not sure how to copy all of the defined
registers, I
only looked briefly but I don't see where that is in the code. I
think I'll
merge in the fix I've been using on my branches for the last few
weeks, and
if nothing breaks, we can call it good for now.

—
Reply to this email directly or view it on GitHub
#36 (comment).

—
Reply to this email directly or view it on GitHub
#36 (comment).

from stoke.

Perfect.

On Mon, Oct 13, 2014 at 1:06 PM, Berkeley Churchill <
[email protected]> wrote:

I've been leaving some around just in case we need to go back for some
reason. But, I've been deleting ones that are over 7 days old that have
been merged in. Does that sound alright?

Berkeley

On 10/13/2014 01:03 PM, eric schkufza wrote:

Sounds good.

Can you also go through the network graph and delete the branches that
you've merged back into develop?

On Mon, Oct 13, 2014 at 1:02 PM, Berkeley Churchill <
[email protected]> wrote:

The tests are here, and none failed, although I can't be certain of how
thorough they are. I'm not sure how to copy all of the defined
registers, I
only looked briefly but I don't see where that is in the code. I
think I'll
merge in the fix I've been using on my branches for the last few
weeks, and
if nothing breaks, we can call it good for now.

—
Reply to this email directly or view it on GitHub
#36 (comment).

—
Reply to this email directly or view it on GitHub
#36 (comment).

—
Reply to this email directly or view it on GitHub
#36 (comment).

from stoke.

This seems all fixed for now. If we discover this introduced a new bug down the road, we can create a new issue.

from stoke.