Re: incorrect disassembly on vliw / little endian

[Patrick Macdonald <patrickm at redhat dot com>]

Hey Doug,

Doug Evans wrote:
> 
> Patrick Macdonald writes:
>  > Patrick Macdonald wrote:
>  > >
>  > > Hello,
>  > >
>  > > This is a follow up to my prior note.  This patch fixes the vliw,
>  > > little endian problem I'm seeing.  I have a few concerns though:
>  > >
>  > > 1. Is it too restrictive?
>  > > 2. Can we assume that most reads/writes will be multiples of 8?
>  >
>  > Missing an "even" there.  Point 2 should read:
>  >
>  >   2. Can we assume that most reads/writes will be even multiples of
>  >      8 (except for 8)?  (8/16/32)
>  >
>  > > Comments/suggestions?  If not, I'll submit it.
> 
> fwiw, this doesn't seem like the right way to go.
> [it may certainly be the best expedient way though]

The patch didn't give me a warm and fuzzy but it got this particular job
done.  

> I'm guessing this is the code that's getting you into trouble.
> Clearly the `then' branch is the wrong thing to do for your port.

I'm not sure it's all that clear.
It would have been fine had it been big endian.  The base insn size
of 16 bits followed by the 16 bit immediate would read in perfectly.
It's just that for the logic to work on little endian, it would have
to be a 16 bit immediate followed by a 16 bit base instruction.  An
alternative is to have read_insn read only the x bytes after the
instruction and place it in proper order inside full_insn_value.
Haven't thought it out much but it looks doable.
 
> [While Frank can certainly disagree, I kinda like having
> the int insns [for sparc, etc.] - one problem is that the support got
> extended to ports that has thus far worked, but with recent ports
> is now starting to break.]
> 
>           /* Make sure the entire insn is loaded into insn_value, if it
>              can fit.  */
>           if (CGEN_INSN_BITSIZE (insn) > cd->base_insn_bitsize &&
>               (CGEN_INSN_BITSIZE (insn) / 8) <= sizeof (unsigned long))
>             {
>               unsigned long full_insn_value;
>               int rc = read_insn (cd, pc, info, buf,
>                                   CGEN_INSN_BITSIZE (insn) / 8,
>                                   & ex_info, & full_insn_value);
>               if (rc != 0)
>                 return rc;
>               length = CGEN_EXTRACT_FN (cd, insn)
>                 (cd, insn, &ex_info, full_insn_value, &fields, pc);
>             }
>           else
>             length = CGEN_EXTRACT_FN (cd, insn)
>               (cd, insn, &ex_info, insn_value, &fields, pc);
> 
> Maybe a better solution lies along getting the `else' branch
> working.

Hmmm... things could get pretty ugly as the else branch is the common case. 

The if-then is for vliw, big endian.  Should we not try to make it for vliw
regardless of the endian?

Patrick