On Wed, Jan 22, 2014 at 02:01:30PM +1100, Mark Brown wrote:
> Hi Paul,
>
> >
> > As mentioned above store(S) (like io) is special and supposed to never
> > actually appear in generated code,
>
> It's never *required* to appear, but it still does. If you pass IO to
> list.foldl, for example, the implementation will copy around a piece
> of garbage for you.
Okay, this is the most compelling reason as to why it's okay to represent
variables of these types in programs, because it's easier with higher order
code.
> > this is true but it's not true when it's
> > existentially typed - it cannot be as the type is abstract. So if there was
> > no register conflict when generating the procedure's epilogue then an
> > arbitrary garbage value would have been saved into MR_r2, and the calling
> > program would pass it around and as long as it was never coerced into the
> > wrong type it would have been safe, it's never actually used or
> > dereferenced. This appears to be a harmless bug,
>
> It's harmless, but not a bug. There's no information here, and garbage
> faithfully represents that.
>
> > it only becomes harmful
> > when a register conflict occurs and the compiler crashes.
>
> Trying to generate code that reads something out of a dummy slot is a bug.
Right.
> >
> > The compiler currently tries to avoid conflicts by copying about-to-be
> > clobbered information out of the way, it uses the existing stack slot if
> > there is one. I modified this behaviour to ignore stack slots with negative
> > numbers, in these cases a new register is used and tried it MR_r5, the
> > compiler happily generates the code below, which will probably run
> > correctly, well equally as correctly as the above bug without a register
> > conflict.
> >
> > /*Start of procedure epilogue*/
> > MR_r1 = MR_sv(1);
> > /* Placing ExistQTypeClassInfo_for_store */
> > MR_r2 = MR_tempr1;
> > /* Placing ExistQEnvironment */
> > MR_r3 = MR_r5;
> > /* Placing ExistQStore */
> > MR_decr_sp_and_return(2);
> > /* Return from procedure call */
> >
> > So my question is, how should this _really_ be fixed. Is a hack like the one
> > I've used 'good enough'?
>
> Possibly. But why not just leave off the assignment to MR_r3 altogether?
>
Skipping the assignment at this point might be tricky. It may be possible
by changing the variable store maps in the LLDS backend to track whether a
variable is actually 'real', if it's not-real then it can simply not
generate any instructions that update that variable (and that variable
alone).
Another idea I had was to turn an exist_cast HLDS goal of something like io
or store(S) into an assignment from NULL (eg: "MR_r3 = NULL"[0]) rather than
an assignment from where-ever we thought that store(S) or io was stored.
NULL is just as good as junk but it has the benefit of being a 'cached'
value (as far as the LLDS is concerned), this means that if all it's
locations get clobbered that we can generate a new value for it from the
constant NULL. No stack slot or temporary register is required. This is
not an improvement on your suggestion above, it's just easier.
Thanks.
0. This is not actually the instruction generated, some other
instruction is generated and then optimized away before the compiler
wrote out the C file.
--
Paul Bone
http://www.bone.id.au