This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: V3: [PATCH] x32: Generate 0x67 prefix for VSIB address without base


On Wed, Feb 27, 2019 at 12:26 AM Jan Beulich <JBeulich@suse.com> wrote:
>
> >>> On 26.02.19 at 21:33, <hjl.tools@gmail.com> wrote:
> > Here is the updated patch.  I added VecSIBQword to mark VSIB instructions
> > with Qword indices and add 0x67 prefix only for VSIB address of Dword
> > indices without base register nor symbol so that Dword indices will be
> > zero-extended to 64 bits unless -moperand-check=none is passed to
> > assembler.
>
> A couple of questions still remain:
>
> 1) What about a scale factor other than 1? Arguably this is difficult to
> use with neither base nor O_symbol displacement, but it's not
> impossible. As said before, _if_ qword indices are to be special cased,
> I think such scale factors should be, too.
>
> 2) Given the wording you had quoted from psABI section 10.4, I did
> suggest that special casing of qword indexes may then not be
> necessary at all. Could you clarify why you (now) think otherwise?
>
> 3) Does the logic work not only with a specified displacement of zero,
> but also without any displacement at all? The abort() invocations you
> add make me uncertain of this, and the test cases you add don't
> cover the case.
>
> 4) Why "else if (i.prefix[ADDR_PREFIX] && i.prefix[SEG_PREFIX])"
> instead of just if()? Isn't this diagnostic equally applicable to all the
> VecSIB cases?
>
> 5) In the comment following "case O_constant", could you add
> "assuming that the index register actually holds addresses" or
> something along these lines? Similarly the other comment is still as
> vague as it was before; as said I really think it lacks sufficient
> clearness as to the "why", i.e. the non-wrapping behavior at 4Gb
> should be mentioned explicitly rather then be implied.
>
> 6) You still use the existing operand_check to control the diagnostic.
> This being a very special case which one may want to disable without
> also disabling diagnostics for other, more generic operand checks,
> don't you agree that it should be separately controllable?
>
> 7) Would you mind addressing the previously raised point of it (in
> my opinion) really being the compiler's / assembly programmer's job
> to enforce 32-bit addressing here?
>

Good point.  I withdraw my patch.  I opened:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89523


-- 
H.J.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]