Skip to content

Fix HLSL conversion warnings and integer conversion correctness - #1091

Open
CrabExtra wants to merge 1 commit into
masterfrom
DXC_warning_fix
Open

CrabExtra wants to merge 1 commit into
masterfrom
DXC_warning_fix

Conversation

@CrabExtra

Copy link
Copy Markdown
Contributor

Description

Testing

TODO list:

Comment on lines 618 to +622
template<class T>
using make_signed = std::make_signed<T>;
struct make_signed : std::make_signed<T> {};

template<class T>
using make_unsigned = std::make_unsigned<T>;
struct make_unsigned : std::make_unsigned<T> {};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thats fine just annotate why its struct and not using


template<>
struct NBL_ADD_STD make_signed<nbl::hlsl::emulated_int64_t> : type_identity<nbl::hlsl::emulated_int64_t> {};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes this is fine

Comment on lines +262 to +266
const int32_t msbSigned = _static_cast<int32_t>(operand.__getMSB());
const int32_t shiftSigned = _static_cast<int32_t>(shift);
const int32_t bitsSigned = _static_cast<int32_t>(bits);
const type_t shifted = type_t::create(bigShift ? vector<uint32_t, 2>(_static_cast<uint32_t>(msbSigned >> shiftSigned), msbSigned < 0 ? ~uint32_t(0) : uint32_t(0))
: vector<uint32_t, 2>((operand.__getMSB() << shift) | (operand.__getLSB() >> bits), _static_cast<uint32_t>(msbSigned >> bitsSigned)));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Przemog1 review that it does the same thing

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like it does

Comment on lines +101 to +102
// Integer conversions use the same round-toward-zero mode as arithmetic.
mantissa = val >> _static_cast<uint64_t>(-shiftCnt);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to cast to uint64, or does compiler complain about you not using a shift operand of the same type ?

Everything should really be int16_t from findMSB onwards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants