This is an archive of the discontinued LLVM Phabricator instance.

Differential D67937

Fix int to bool errors exposed due to r372612
ClosedPublic

Authored by rdhindsa on Sep 23 2019, 3:33 PM.

Details

Reviewers
rupprecht
xbolva00
Commits
rG1605eb1c1cfc: Fix int to bool errors exposed due to r372612.
rCRT372684
rL372684
Summary

Fix int to bool conversions exposed by r372612. ( Warn if '<<' in bool context with -Wint-in-bool-context (GCC compatibility))

Diff Detail

Event Timeline

rdhindsa created this revision.Sep 23 2019, 3:33 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 23 2019, 3:33 PM
Herald added a subscriber: llvm-commits. · View Herald Transcript
rupprecht added a reviewer: xbolva00.Sep 23 2019, 3:57 PM
xbolva00 accepted this revision.Sep 23 2019, 3:58 PM
This revision is now accepted and ready to land.Sep 23 2019, 3:58 PM
rupprecht added inline comments.Sep 23 2019, 4:00 PM
compiler-rt/lib/builtins/fp_add_impl.inc
97

nit: I think there is an extra unnecessary set of parens, e.g.

const bool sticky = (bSignificand << (typeWidth - align)) != 0;

(same for the other cases)

xbolva00 added inline comments.Sep 23 2019, 4:11 PM
compiler-rt/lib/builtins/fp_add_impl.inc
97

+1, yes, please fix them

rdhindsa updated this revision to Diff 221441.Sep 23 2019, 4:58 PM

Removed the extra parenthesis.

rdhindsa marked 2 inline comments as done.Sep 23 2019, 4:59 PM
Closed by commit rL372684 (authored by rdhindsa). · Explain WhySep 23 2019, 7:58 PM
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: delcypher. · View Herald TranscriptSep 23 2019, 7:58 PM

Revision Contents

PathSize
compiler-rt/
lib/
builtins/
4 lines
2 lines
2 lines

Diff 221441

compiler-rt/lib/builtins/fp_add_impl.inc

Context not available.
const unsigned int align = aExponent - bExponent; const unsigned int align = aExponent - bExponent;
if (align) { if (align) {
if (align < typeWidth) { if (align < typeWidth) {
const bool sticky = bSignificand << (typeWidth - align); const bool sticky = (bSignificand << (typeWidth - align)) != 0;
rupprechtUnsubmitted
Done
ReplyInline Actions

nit: I think there is an extra unnecessary set of parens, e.g.

const bool sticky = (bSignificand << (typeWidth - align)) != 0;

(same for the other cases)

rupprecht: nit: I think there is an extra unnecessary set of parens, e.g. ``` const bool sticky =…
xbolva00Unsubmitted
Done
ReplyInline Actions

+1, yes, please fix them

xbolva00: +1, yes, please fix them
bSignificand = bSignificand >> align | sticky; bSignificand = bSignificand >> align | sticky;
} else { } else {
bSignificand = 1; // Set the sticky bit. b is known to be non-zero. bSignificand = 1; // Set the sticky bit. b is known to be non-zero.
Context not available.
// The result is denormal before rounding. The exponent is zero and we // The result is denormal before rounding. The exponent is zero and we
// need to shift the significand. // need to shift the significand.
const int shift = 1 - aExponent; const int shift = 1 - aExponent;
const bool sticky = aSignificand << (typeWidth - shift); const bool sticky = (aSignificand << (typeWidth - shift)) != 0;
aSignificand = aSignificand >> shift | sticky; aSignificand = aSignificand >> shift | sticky;
aExponent = 0; aExponent = 0;
} }
Context not available.

compiler-rt/lib/builtins/fp_lib.h

Context not available.
static __inline void wideRightShiftWithSticky(rep_t *hi, rep_t *lo, static __inline void wideRightShiftWithSticky(rep_t *hi, rep_t *lo,
unsigned int count) { unsigned int count) {
if (count < typeWidth) { if (count < typeWidth) {
const bool sticky = *lo << (typeWidth - count); const bool sticky = (*lo << (typeWidth - count)) != 0;
*lo = *hi << (typeWidth - count) | *lo >> count | sticky; *lo = *hi << (typeWidth - count) | *lo >> count | sticky;
*hi = *hi >> count; *hi = *hi >> count;
} else if (count < 2 * typeWidth) { } else if (count < 2 * typeWidth) {
Context not available.

compiler-rt/lib/builtins/fp_trunc_impl.inc

Context not available.
if (shift > srcSigBits) { if (shift > srcSigBits) {
absResult = 0; absResult = 0;
} else { } else {
const bool sticky = significand << (srcBits - shift); const bool sticky = (significand << (srcBits - shift)) != 0;
src_rep_t denormalizedSignificand = significand >> shift | sticky; src_rep_t denormalizedSignificand = significand >> shift | sticky;
absResult = denormalizedSignificand >> (srcSigBits - dstSigBits); absResult = denormalizedSignificand >> (srcSigBits - dstSigBits);
const src_rep_t roundBits = denormalizedSignificand & roundMask; const src_rep_t roundBits = denormalizedSignificand & roundMask;
Context not available.