llvm dev - Oct 2015 - [compiler-rt] Undefined negation in float emulation functions

On 26/10/15 02:54, Joerg Sonnenberger wrote:
> On Sun, Oct 25, 2015 at 06:46:48AM -0400, Steve Canon via llvm-dev wrote:
>> On Oct 24, 2015, at 6:02 PM, Chris Lattner <clattner at
apple.com> wrote:
>>>
>>>
>>>> On Oct 23, 2015, at 7:43 PM, Matthew Fernandez via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>>>>> On 21/10/15 00:15, Stephen Canon wrote:
>>>>> Yup, this is UB.  If you want to propose a patch, I would
do something like the following:
>>>>>
>>>>>     rep_t sign = 0;
>>>>>     unsigned int aAbs = a;
>>>>>     if (a < 0) {
>>>>>         sign = signBit;
>>>>>         aAbs = -aAbs;
>>>>>     }
>>>>>     // Now use aAbs instead of a.
>>>
>>> FWIW, another way to avoid the UB is to use an unsigned value.
>>
>> I'm confused, that's exactly what this does.
>
> At least in one place it is negating the signed variable, not creating a
> new unsigned variable.
Chris, were you suggesting a cast (`a = -(unsigned)a`)? Otherwise, like Steve, I
don't understand the difference between
this proposal and Steve's code above.

On 26 October 2015 at 07:56, Matthew Fernandez
<matthew.fernandez at gmail.com> wrote:
> On 26/10/15 02:54, Joerg Sonnenberger wrote:
>>
>> On Sun, Oct 25, 2015 at 06:46:48AM -0400, Steve Canon via llvm-dev
wrote:
>>>
>>> On Oct 24, 2015, at 6:02 PM, Chris Lattner <clattner at
apple.com> wrote:
>>>>
>>>>
>>>>
>>>>> On Oct 23, 2015, at 7:43 PM, Matthew Fernandez via llvm-dev
>>>>> <llvm-dev at lists.llvm.org> wrote:
>>>>>>
>>>>>> On 21/10/15 00:15, Stephen Canon wrote:
>>>>>> Yup, this is UB.  If you want to propose a patch, I
would do something
>>>>>> like the following:
>>>>>>
>>>>>>     rep_t sign = 0;
>>>>>>     unsigned int aAbs = a;
>>>>>>     if (a < 0) {
>>>>>>         sign = signBit;
>>>>>>         aAbs = -aAbs;
>>>>>>     }
>>>>>>     // Now use aAbs instead of a.
>>>>
>>>>
>>>> FWIW, another way to avoid the UB is to use an unsigned value.
>>>
>>>
>>> I'm confused, that's exactly what this does.
>>
>>
>> At least in one place it is negating the signed variable, not creating
a
>> new unsigned variable.
>
>
> Chris, were you suggesting a cast (`a = -(unsigned)a`)? Otherwise, like
> Steve, I don't understand the difference between this proposal and
Steve's
> code above.
I started an attempt at this today, but the current revision, r251669,
doesn't seem to build for me. I don't recall having problems building
Clang in the past and this time I just followed the standard getting
started instructions [0] without any odd flags in my environment. The
actual error is pages long, but is essentially a link failure when
forming libc++.so caused by failing to find vtables for things like
`__cxxabiv1::__si_class_type_info`. Is this a known issue? If the
solution is not straightforward, perhaps I should move this over to
Bugzilla.

  [0]: http://clang.llvm.org/get_started.html

On 24/10/15 13:43, Matthew Fernandez wrote:
 > ... My intended approach is (1) propose a patch that avoids the signed
negation in __floatsidf
 > and __floatsisf as you suggested, (2) leave test__absv.i2 as-is as INT_MIN
as an input is
 > documented to be undefined, and (3) propose a patch that rephrases the
left shifts in __absv.i2
 > to avoid UB there.

I've posted a version of (1) to llvm-commits and CCed the participants of
this thread. If all goes well, I'll send
another patch for (3).

On 30/10/15 11:46, Matthew Fernandez wrote:
>
> I started an attempt at this today, but the current revision, r251669,
> doesn't seem to build for me. I don't recall having problems
building
> Clang in the past and this time I just followed the standard getting
> started instructions [0] without any odd flags in my environment. The
> actual error is pages long, but is essentially a link failure when
> forming libc++.so caused by failing to find vtables for things like
> `__cxxabiv1::__si_class_type_info`. Is this a known issue? If the
> solution is not straightforward, perhaps I should move this over to
> Bugzilla.
>
>    [0]: http://clang.llvm.org/get_started.html
It turned out my confusion here was because I had cloned libcxx but not
libcxxabi into llvm/projects. The LLVM getting
started docs [1] give you this pointer, but the above Clang getting started docs
only reference libcxx. Is this an
omission on the Clang site or did I somehow manage to select a feature I
shouldn't have? This was on an x86_64 Linux box
where I followed the Clang instructions exactly as written, so I'd be
surprised if this was the case.

   [1]: http://llvm.org/docs/GettingStarted.html