This is an archive of the discontinued LLVM Phabricator instance.

[libc] Support constexpr uint initialization
ClosedPublic

Authored by michaelrj on Apr 19 2023, 4:55 PM.

Details

Summary

Uint addition and subtraction normally use builtins which aren't
constexpr. This patch adds an rvalue overload version of the addition
and subtraction operation that is always constexpr.

Diff Detail

Event Timeline

michaelrj created this revision.Apr 19 2023, 4:55 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptApr 19 2023, 4:55 PM
michaelrj requested review of this revision.Apr 19 2023, 4:55 PM

Looks pretty good but I have some minor questions.

libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
51–52

Why was this change required?

libc/src/__support/UInt.h
86

Is this required for the functionality of this change?

94

Is this required for the functionality of this change? Also, wouldn't a more correct check be *this != 0?

libc/src/__support/builtin_wrappers.h
88

Incomplete sentence?

Thanks, it fixed the problem with sub_with_borrow and add_with_carry in riscv32 for me!

The extra operators also helped with the compilation and only one extra change was required to get libc built for riscv32:

diff --git a/libc/src/__support/FPUtil/NearestIntegerOperations.h b/libc/src/__support/FPUtil/NearestIntegerOperations.h
index 06aa9484c3f7..b9332528f60c 100644
--- a/libc/src/__support/FPUtil/NearestIntegerOperations.h
+++ b/libc/src/__support/FPUtil/NearestIntegerOperations.h
@@ -133,7 +133,7 @@ LIBC_INLINE T round(T x) {
   }
 
   uint32_t trim_size = MantissaWidth<T>::VALUE - exponent;
-  bool half_bit_set = bits.get_mantissa() & (UIntType(1) << (trim_size - 1));
+  bool half_bit_set = bool(bits.get_mantissa() & (UIntType(1) << (trim_size - 1)));
   bits.set_mantissa((bits.get_mantissa() >> trim_size) << trim_size);
   T trunc_value = T(bits);

If the operators are accepted in this patch, do you mind adding this one change as well?

libc/src/__support/UInt.h
94

I did something similar to get compilation working in riscv32, but as:

constexpr explicit operator bool() const { return !is_zero(); }

I think Operator== will construct a UInt(1), while is_zero doesn't

michaelrj marked 4 inline comments as done.

address comments

libc/src/__support/FPUtil/x86_64/LongDoubleBits.h
51–52

This change isn't required, it's from when I was trying to move the constants away from addition and subtraction. That being said, I do think leaving this change would be good because we could remove an extra set of parentheses.

libc/src/__support/UInt.h
86

yes, because without this the casts in LongDoubleBits don't work with UInt.

A related question might be "why are there casts in LongDoubleBits?" The answer is I was testing the constant functionality by using UInt<128> instead of the builtin uint_128 on my x86_64 workstation.

sivachandra accepted this revision.Apr 20 2023, 11:52 AM
This revision is now accepted and ready to land.Apr 20 2023, 11:52 AM
This revision was landed with ongoing or failed builds.Apr 20 2023, 1:09 PM
This revision was automatically updated to reflect the committed changes.