This is an archive of the discontinued LLVM Phabricator instance.

Revert "Revert "Fix handling of special and large vals in expand pattern for `round`" and "Add pattern that expands `math.roundeven` into `math.round` + arith""
ClosedPublic

Authored by ramiro050 on Apr 21 2023, 9:59 AM.

Details

Summary

This reverts commit 87cef78fa1c7bf6efc544e990894a6062d56abec.

The issue in the original revert is that a lit test expecting a -nan
as an output was failing on M2. Since the IEEE 754-2008 standard does
not require the sign to be printed when displaying a nan, this
commit changes the CHECK for -nan to one that checks the result
value bitcasted to an i32 to ensure that input is being left
unchanged. This check should now be independent of platform being used
to run test.

Diff Detail

Event Timeline

ramiro050 created this revision.Apr 21 2023, 9:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 21 2023, 9:59 AM
ramiro050 requested review of this revision.Apr 21 2023, 9:59 AM

Can we test for the sign and print it differently? For example extracting the sign in an i1 and printing the i1

Can we test for the sign and print it differently? For example extracting the sign in an i1 and printing the i1

Yeah, I can do that. Alternatively, I can just print the result bitcasted to int and compare that. This will ensure that the value is actually left unchanged by the pattern. I did this for testing large numbers to avoid scientific notation being used for printing.

Change check for -nan to a check that looks at the result bitcasted as an i32.

ramiro050 edited the summary of this revision. (Show Details)Apr 21 2023, 11:05 AM

Updated tests for positive nan to also use bitcast to i32, since checking just for nan would fail to catch the case where the function erroneously outputs -nan.

This revision is now accepted and ready to land.Apr 21 2023, 12:21 PM
jpienaar accepted this revision.Apr 22 2023, 6:15 AM

Thanks, I'll help land later today