Skip to content

Serialize negative constants according to the operand type #118814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

Serialize negative constants according to the operand type #118814

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9a24051
Serialize negative constants according to the operand type
ashay Aug 16, 2025
3831af7
Unify logic for printing both positive and negative values
ashay Aug 18, 2025
4686534
Fixed catch-all case to use `size_t` instead of `ssize_t`
ashay Aug 18, 2025
98ce373
Retain existing behavior when printing values between -1000 and +1000
ashay Aug 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion src/coreclr/jit/emitxarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12648,10 +12648,35 @@ void emitter::emitDispConstant(const instrDesc* id, bool skipComma) const
{
printf("%d", (int)val);
Copy link
Contributor

@xtqqczze xtqqczze Aug 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

we were printing decimal here not hex

Copy link
Author

Choose a reason for hiding this comment

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

Yikes! Good catch. Fixed.

}
else if ((val > 0) || (val < -0xFFFFFF))
else if (val > 0)
{
printf("0x%zX", (ssize_t)val);
}
else if (val < -0xFFFFFF)
{
switch (id->idOpSize())
{
case EA_1BYTE:
printf("0x%X", static_cast<int8_t>(val));
break;

case EA_2BYTE:
printf("0x%X", static_cast<int16_t>(val));
break;

case EA_4BYTE:
printf("0x%X", static_cast<int32_t>(val));
break;

case EA_8BYTE:
printf("0x%X", static_cast<int64_t>(val));
Copy link

Choose a reason for hiding this comment

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

This is not correct if int is 32-bit. %X expects an unsigned int argument but int64_t may be wider than that.

I think you can use "0x%zX" with (size_t)(uint64_t)val. I don't see why one would use the signed type ssize_t with %zX.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for catching that! I had missed that %X expects an unsigned integer. Instead of printing the integers with the sign, I unified the logic for printing both positive and negative numbers by printing the unsigned equivalent. This has the added benefit of making the code simpler, IMO.

break;

default:
printf("0x%zX", (ssize_t)val);
break;
}
}
else
{
// (val < 0)
Expand Down
Loading