On Thu, Sep 5, 2024 at 3:10 PM Jeff Law <jeffreya...@gmail.com> wrote:
> On 9/5/24 6:16 AM, Raphael Zinsly wrote:
> > On Wed, Sep 4, 2024 at 8:32 PM Jeff Law <j...@ventanamicro.com> wrote:
> >> On 9/2/24 2:01 PM, Raphael Moreira Zinsly wrote:
> >> ...
> >>> +      bool bit31 = (hival & 0x80000000) != 0;
> >>> +      int trailing_shift = ctz_hwi (loval) - ctz_hwi (hival);
> >>> +      int leading_shift = clz_hwi (loval) - clz_hwi (hival);
> >>> +      int shiftval = 0;
> >>> +
> >>> +      /* Adjust the shift into the high half accordingly.  */
> >>> +      if ((trailing_shift > 0 && hival == (loval >> trailing_shift))
> >>> +       || (trailing_shift < 0 && hival == (loval << trailing_shift)))
> >>> +     shiftval = 32 - trailing_shift;
> >>> +      else if ((leading_shift < 0 && hival == (loval >> leading_shift))
> >>> +             || (leading_shift > 0 && hival == (loval << leading_shift)))
> >> Don't these trigger undefined behavior when tailing_shift or
> >> leading_shift is < 0?  We shouldn't ever generate negative shift counts.
> >
> > The value of trailing/leading_shift is added to 32, we will never have
> > negative shift counts.
> In the IF you have this conditional:
>
> > (trailing_shift < 0 && hival == (loval << trailing_shift))
>
> How could that not be undefined behvaior?  You first test that the value
> is less than zero and if it is less than zero you use it as a shift count.

I'm not using trailing_shift as the shift count, I'm using shiftval:

+         /* Now we want to shift the previously generated constant into the
+            high half.  */
+         alt_codes[alt_cost - 2].code = ASHIFT;
+         alt_codes[alt_cost - 2].value = shiftval;
+         alt_codes[alt_cost - 2].use_uw = false;
+         alt_codes[alt_cost - 2].save_temporary = false;


-- 
Raphael Moreira Zinsly

Reply via email to