http://bugzilla.gdcproject.org/show_bug.cgi?id=57

Johannes Pfau <johannespfau@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |johannespfau@gmail.com

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.gdcproject.org/show_bug.cgi?id=57

--- Comment #1 from Iain Buclaw <ibuclaw@gdcproject.org> 2013-05-28 20:17:30 UTC ---
Incidentally, I actually thought about this earlier when looking at another test using empty structs. The fix didn't work for that, but it just so happens that it works for this...

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.gdcproject.org/show_bug.cgi?id=57

--- Comment #2 from Iain Buclaw <ibuclaw@gdcproject.org> 2013-05-28 20:24:17 UTC ---
Can't post patch here as it's on another computer but it's essentially:

-- AssignExp::toElem
remove sle->fillHoles code.


-- StructLiteralExp::toElem
Put fillHoles code here and create temp var so codegen turns into.

struct tmp;
memset (&tmp, sizeof(struct), 0), tmp = { ctor }, tmp;

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.gdcproject.org/show_bug.cgi?id=57

Iain Buclaw <ibuclaw@gdcproject.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #3 from Iain Buclaw <ibuclaw@gdcproject.org> 2013-05-28 20:48:26 UTC ---
https://github.com/D-Programming-GDC/GDC/commit/f7569e041ea924f6cecf38cb0e3192a3cf187e2f

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.gdcproject.org/show_bug.cgi?id=57

--- Comment #4 from Iain Buclaw <ibuclaw@gdcproject.org> 2013-05-28 21:55:44 UTC ---
Update:

https://github.com/D-Programming-GDC/GDC/commit/58f03823aeb72109a69673f111092548aca081f5


Actually got both cases in one hit this time...

---
static struct Foo
{
    static Foo opCall(int n, int m) { Foo foo; return foo; }
}


Foo foo;
assert (foo(1,2) == Foo.init);
---

The above was failing for 32bit only, with optimisations disabled.  (See
opover3.d).

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.gdcproject.org/show_bug.cgi?id=57

Johannes Pfau <johannespfau@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |NEW
         Resolution|FIXED                       |

--- Comment #5 from Johannes Pfau <johannespfau@gmail.com> 2013-05-29 17:26:17 UTC ---
Nice work!

But I can still reproduce this with the posted test case (GCC 4.8.0) if I compile without optimization. With -O2 or higher it works fine.

Now the struct is initialized correctly in funcA:
movq   $0x0,(%rax)
movq   $0x0,0x8(%rax)
movl   $0x4,-0x18(%rbp)
movq   $0x2a,-0x10(%rbp)

And returned:
mov    -0x18(%rbp),%rax
mov    -0x10(%rbp),%rdx

But in the main function:
mov    %eax,%ecx
mov    %rdx,%rax
mov    %ecx,-0x30(%rbp) <--- uses only 4 bytes of rax
                        //-0x2C(%rbp) is not initialized
mov    %rax,-0x28(%rbp)


BTW: Do we also have to add the fillHoles code to StructInitializer::toDt?

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.gdcproject.org/show_bug.cgi?id=57

--- Comment #6 from Iain Buclaw <ibuclaw@gdcproject.org> 2013-05-29 17:56:36 UTC ---
(In reply to comment #5)
> Nice work!
> 
> But I can still reproduce this with the posted test case (GCC 4.8.0) if I compile without optimization. With -O2 or higher it works fine.
> 

Drats - should have tried that test case....


> 
> BTW: Do we also have to add the fillHoles code to StructInitializer::toDt?

Shouldn't be needed because of the dt_zeropad that pads out the rest of the structure.  Though we can certainly skip over the logic in that routine by checking if sd->zeroInit.

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.gdcproject.org/show_bug.cgi?id=57

--- Comment #7 from Iain Buclaw <ibuclaw@gdcproject.org> 2013-05-29 18:01:29 UTC ---
(In reply to comment #6)
> (In reply to comment #5)
> > Nice work!
> > 
> > But I can still reproduce this with the posted test case (GCC 4.8.0) if I compile without optimization. With -O2 or higher it works fine.
> > 
> 
> Drats - should have tried that test case....
> 

I actually might have an idea of where that is happening (see convert_for_assignment) - can't test it just now as I'm travelling.  But I wonder what happens if you move the memset added in AssignExp::toElem into there - so that all struct = int (0) code is caught and handled properly...

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.gdcproject.org/show_bug.cgi?id=57

--- Comment #8 from Iain Buclaw <ibuclaw@gdcproject.org> 2013-05-29 19:56:13 UTC ---
(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > Nice work!
> > > 
> > > But I can still reproduce this with the posted test case (GCC 4.8.0) if I compile without optimization. With -O2 or higher it works fine.
> > > 
> > 
> > Drats - should have tried that test case....
> > 
> 
> I actually might have an idea of where that is happening (see convert_for_assignment) - can't test it just now as I'm travelling.  But I wonder what happens if you move the memset added in AssignExp::toElem into there - so that all struct = int (0) code is caught and handled properly...

Done!

https://github.com/D-Programming-GDC/GDC/commit/ed839f8a16e84d0eddaac3100dae4905ebcd58e9

I actually couldn't reproduce the problem in my tree... But this is a better fix anyway as it catches all cases.

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.gdcproject.org/show_bug.cgi?id=57

--- Comment #9 from Johannes Pfau <johannespfau@gmail.com> 2013-05-30 07:45:04 UTC ---
Unfortunately I can still reproduce it.

Using GCC 4.8.0, GDC revision ed839f8a16e84d0eddaac3100dae4905ebcd58e9. Happens only without optimization and only on 64 bit.

Compiled using "/opt/gdc/bin/gdc smallstruct.d"

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.