Re: [PATCH 3/6] x86: harmonize disp with imm handling

["H.J. Lu" <hjl.tools@gmail.com>]

On Sun, Jun 20, 2021 at 11:36 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 18.06.2021 17:41, H.J. Lu wrote:
> > On Fri, Jun 18, 2021 at 7:52 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 18.06.2021 16:12, H.J. Lu wrote:
> >>> On Fri, Jun 18, 2021 at 2:03 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>
> >>>> On 17.06.2021 18:12, H.J. Lu wrote:
> >>>>> On Thu, Jun 17, 2021 at 9:05 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>
> >>>>>> On 17.06.2021 18:00, H.J. Lu wrote:
> >>>>>>> On Thu, Jun 17, 2021 at 7:57 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>>>
> >>>>>>>> On 17.06.2021 16:46, H.J. Lu wrote:
> >>>>>>>>> On Mon, Jun 14, 2021 at 3:25 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>>>>>>>>> --- /dev/null
> >>>>>>>>>> +++ b/gas/testsuite/gas/i386/disp-imm-32.s
> >>>>>>>>>> @@ -0,0 +1,17 @@
> >>>>>>>>>> +       .text
> >>>>>>>>>> +disp_imm:
> >>>>>>>>>> +       mov     -0xffffffff(%eax), %eax
> >>>>>>>>>
> >>>>>>>>> I don't think we should treat  -0xffffffff(%eax) as 1(%eax).
> >>>>>>>>> We allow addresses to wraparound. I don't see a need for
> >>>>>>>>> displacements to wraparound.
> >>>>>>>>
> >>>>>>>> This then is entirely unexpected to the programmer. In fact the
> >>>>>>>> same (abstracted away behind some defines or equates) constant
> >>>>>>>> could be used for both purposes (and should be usable both ways,
> >>>>>>>> imo).
> >>>>>>>
> >>>>>>> Since hardware wraparound on DISP + BASE + INDEX * SCALE, not
> >>>>>>> on DISP, it is wrong to change DISP + BASE + INDEX * SCALE to
> >>>>>>> wraparound (DISP) + BASE + INDEX * SCALE.
> >>>>>>
> >>>>>> But this is true regardless of how small (or big) the displacement.
> >>>>>> Without knowing the register values, you can't know at what
> >>>>>> displacement values wraparound occurs. Also, unless I'm mistaken,
> >>>>>> wrapround(a + b) == wrapround(wrapround(a) + wrapround(b)).
> >>>>>
> >>>>> Hardware does wraparound (DISP + BASE + INDEX * SCALE).
> >>>>> Assembler and linker should only wraparound on the final address.
> >>>>
> >>>> I'm afraid this last sentence makes no sense to me: The assembler
> >>>> (or linker) can't know the final address. Instead, both immediates
> >>>> and displacements should allow for anything the programmer might
> >>>> sensibly use. If 0xffffffff as a displacement is fine (meaning
> >>>> -1 really), -0xffffffff (meaning 1) ought to be, too. Or else
> >>>> where do you draw the boundary of which displacements are
> >>>> "legitimate" and which are not?
> >>>
> >>> If the program wants 1, use 1.
> >>
> >> You realize that for the purpose of the test case the -0xffffffff is
> >> an over-simplification of what might appear in "real" code, don't
> >> you? It also feels as if you didn't really read my earlier remark:
> >
> > I don't think treating -0xffffffff as 1 here helps anyone.
>
> I'm afraid I was utterly mislead by your commentary here. There's no
> change from this patch in the numeric meaning of -0xffffffff. The
> change here is what warnings result and what encodings get used. As
> the description says:
>
> 'Certain disp values may trigger "... shortened to ..." warnings when
>  equivalent imm ones don't. In some of the cases there are also
>  differences (for non-64-bit code) between BFD64 and !BFD64 builds. The
>  resulting encodings (i.e. use [or not] of the shorter disp8 / imm8
>  forms) are also different in some cases. Make this handling consistent.'
>
> Please may I ask that you take the new testcase and observe the
> difference in behavior with / without BFD64 prior to this patch, and
> the now consistent one?

Make disp consistent is good.  But wraparound disp is not.   Please
find another way to make disp consistent.

-- 
H.J.