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

Andrei Alexandrescu <andrei@erdani.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andrei@erdani.com

--- Comment #5 from Andrei Alexandrescu <andrei@erdani.com> 2014-02-12 07:05:59 GMT ---
$200 bouty placed: https://www.bountysource.com/issues/1407206-arm-runnable-arrayop-d-fails-wrong-execution-order

-- 
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=8

--- Comment #6 from Johannes Pfau <johannespfau@gmail.com> 2014-02-13 08:17:33 GMT ---
Iain, do you want to look into this again?

I read some of the old discussions and the dlang.org pull request and to
summarize:
* All functions should be evaluated LTR, even extern(C)
* Array OPs expect extern(C) functions to be evaluated RTL
* DMD backend does extern(C) functions RTL

So in order to fix this, we'd have to evaluate extern(C) function LTR (easy, as the hard work has already been done for D functions) and change array ops to work with LTR extern(C) functions (needs quite some changes in the frontend and druntime, but it's not hard to implement). However, as soon as we change array ops in the frontend we break RTL backends(dmd, probably ldc). And fixing these backends as well is probably out of scope. We're probably not familiar with these and it might be a bad idea from a legal perspective as well?

However, I think we could do this:
* Make extern(C) functions LTR in GDC
* Make array ops expect LTR C functions in the frontend and druntime
* Introduce a Target::isExternCLTR flag
* Add ~10 lines of special code for backends which use RTL: Just evaluate the
  parameters to VarDecls explicitly before passing them to the C function for
  array ops

This way everything works for all compilers and as soon as all compilers have LTR evaluation we could remove Target::isExternCLTR and the extra code.

-- 
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=8

--- Comment #7 from Johannes Pfau <johannespfau@gmail.com> 2014-02-13 08:18:34 GMT ---
Another link for reference: Pull request for the spec which demands LTR
evaluation:
https://github.com/D-Programming-Language/dlang.org/pull/6

-- 
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=8

--- Comment #8 from Johannes Pfau <johannespfau@gmail.com> 2014-02-13 08:23:04 GMT ---
And one more link: DMD bug: https://d.puremagic.com/issues/show_bug.cgi?id=6620

-- 
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=8

--- Comment #9 from Iain Buclaw <ibuclaw@gdcproject.org> 2014-02-13 08:30:13 GMT ---
LTR is already enforced for LINKd functions.  See d-codegen.cc(d_build_call)

// Evaluate the argument before passing to the function.
// Needed for left to right evaluation.
if (tf->linkage == LINKd && TREE_SIDE_EFFECTS (targ))
  {
    /* ... */
  }

-- 
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=8

--- Comment #10 from Iain Buclaw <ibuclaw@gdcproject.org> 2014-02-13 08:37:03 GMT ---
(In reply to comment #6)
> Iain, do you want to look into this again?
> 
> I read some of the old discussions and the dlang.org pull request and to
> summarize:
> * All functions should be evaluated LTR, even extern(C)
> * Array OPs expect extern(C) functions to be evaluated RTL
> * DMD backend does extern(C) functions RTL
> 
> So in order to fix this, we'd have to evaluate extern(C) function LTR (easy, as the hard work has already been done for D functions) and change array ops to work with LTR extern(C) functions (needs quite some changes in the frontend and druntime, but it's not hard to implement). However, as soon as we change array ops in the frontend we break RTL backends(dmd, probably ldc). And fixing these backends as well is probably out of scope. We're probably not familiar with these and it might be a bad idea from a legal perspective as well?
> 

Not sure about a legal perspective, but if there's something fundamentally wrong with language semantics and we send a fix upstream to the frontend, I'd rely on the respective backend devs to be aware of it, and fix the problem themselves.  It should be easy enough, as all you are doing is caching values in the codegen before passing.

IMO, I think Andrei would agree that all parameters should be LTR evaluated regardless of LINK declarations.


To summarise the current situation:

extern(D) int foo(int a, ...);
extern(C) int bar(int a, ...);

foo(i++, i++, i++);   // 1, 2, 3 - Always!
bar(i++, i++, i++);   // 3, 2, 1 - But only on x86!

-- 
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=8

--- Comment #11 from Johannes Pfau <johannespfau@gmail.com> 2014-02-13 17:25:20 GMT ---
OK. So do you want to fix this or shall I?

-- 
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=8

--- Comment #12 from Andrei Alexandrescu <andrei@erdani.com> 2014-02-13 18:15:27 GMT ---
Yes, I agree all parameters should be evaluated LTR. In fact LTR is more general than that - in an assignment e1 = e2, e1 should be evaluated first. Also in a computed function call expr1(expr2), expr1 should be evaluated first.

-- 
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=8

--- Comment #13 from Iain Buclaw <ibuclaw@gdcproject.org> 2014-02-13 19:21:54 GMT ---
(In reply to comment #12)
> Yes, I agree all parameters should be evaluated LTR. In fact LTR is more general than that - in an assignment e1 = e2, e1 should be evaluated first. Also in a computed function call expr1(expr2), expr1 should be evaluated first.

OK, let's break DMD! >:)

-- 
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=8

--- Comment #14 from Iain Buclaw <ibuclaw@gdcproject.org> 2014-02-13 19:57:41 GMT ---
(In reply to comment #11)
> OK. So do you want to fix this or shall I?

I'm pretty tied up getting gdb D into shape, and working with a publishing company in reviewing a new D2 book, and doing the 2.065 merge - which depends on preliminary stuff such as eventual Visitor conversions almost *all* our glue code, and re-implement dfrontend/builtins.c into the glue cause the new revision of it in 2.065 is not GDC-friendly...


The long route in GDC would be (doing this in my head):

1. d-codegen.cc(d_build_call): Lift the LINKd restriction in which caches
arguments in a LTR fashion.

2. dfrontend/arrayops.c(AssignExp::buildArrayIdent, BinAssignExp::buildArrayIdent, AssignExp::buildArrayLoop, BinAssignExp::buildArrayLoop): Evaluate assign expressions left to right

3. Reverse the parameters for all arrayops in druntime.

// a[] = b[] op value:
(T[] a, T value, T[] b)  =>  (T[] b, T value, T[] a)

// a[] = b[] op c[]
(T[] a, T[] c, T[] b)  =>  (T[] b, T[] c, T[] a)

// a[] += value
(T[] a, value)  =>  (value, T[] a)

// a[] += b[]
(T[] a, T[] b)  =>  (T[] b, T[] a)

4. Test!

5. Push upstream, where someone will work out how to fix it in DMD.


Unless Mr. Andrei thinks that a[] = b[] op c[] should be evaluated in order of: a[] -> b[] -> c[]

Instead of keeping the current behaviour:
b[] -> c[] -> a[]

In which case, just do items number #1, #4 and #5 in the list above.  With the additional step of at stage #4 fix old tests that relied on the old behaviour of evaluation.

Regards
Iain.

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