This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC 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: [Patch] sext_hwi: Avoid left shift of negative value undefined behaviour


Le 12/08/2015 22:07, Richard Sandiford a Ãcrit :
Jeff Law <law@redhat.com> writes:
On 08/12/2015 12:32 PM, Richard Biener wrote:
On August 12, 2015 8:07:13 PM GMT+02:00, Jeff Law <law@redhat.com> wrote:
On 08/12/2015 11:12 AM, Richard Biener wrote:


Prec is almost never a constant and is heavily used from wide-int.

We are not exploiting this undefined ness in C so I object to making
this so much slower.

Can we instead do what we do for abs_hwi and add a checking assert so
we can move the tests to the callers that misbehave instead?
Given that ISO C++ is moving away from making shifting 1 into the sign
bit undefined behaviour, maybe we should make UBSan less strict in its
warning.  That may eliminate the need for Mikael's patch.

We can also use an logical left shift followed by an arithmetic right
shift. Or is the latter invoking undefined behaviour as well in some
cases we hit?
Hmm, why aren't we using logicals for the left shift to begin with?
That's the problem area.  I don't think the right shifts are an issue at
all.

Well, they're implementation-defined, at least in C.  The C11 wording
for E1 >> E2 is "If E1 has a signed type and a negative value, the
resulting value is implementation-defined".  Is C++ different?
(I don't have the standard handy.)

So...

It's strange that when I was researching this, consistently I saw folks
suggesting the LEFT-SHIFT followed by RIGHT-SHIFT, then it'd get shot
down as UB, then folks went to either Mikael's approach or another that
is very similar.  Nobody suggested twidding the types to get a
left-logical-shift, then doing a right-arithmetic-shift.

...unless C++ is different, there's not a standard-level concept
of arithmetic shift.

Still, implementation-defined behavior is a progress over undefined behaviour. Even if it's not set in stone, the good thing about implementation-defined behaviour is that it's known to be non-random.
So it can be checked, either with autoconf, or with a macro.
Would it be acceptable to have both variants of the code and decide among them with such a check? It feels a bit overkill for such a little function, but it is the only way that I see to achieve both need for speed and for well-defined-ness.
I guess it won't kill the bootstrap-ubsan errors though.

Mikael


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