This is an archive of the discontinued LLVM Phabricator instance.

[test-suite][tsvc] Use fabs in s318 and s3113
ClosedPublic

Authored by SjoerdMeijer on Nov 29 2022, 7:07 AM.

Details

Summary

In the s318 and s3113 kernels, the integer variant of the abs() function was to determined the absolute value. It operates on a float value, and the result is stored in a variable max which has the float type, which makes me think the intention here was to use fabs() variant, not the integer abs() variant.

Diff Detail

Event Timeline

SjoerdMeijer created this revision.Nov 29 2022, 7:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 29 2022, 7:07 AM
SjoerdMeijer requested review of this revision.Nov 29 2022, 7:07 AM
SjoerdMeijer added inline comments.Nov 29 2022, 7:09 AM
MultiSource/Benchmarks/TSVC/types.h
12

This is not entirely ideal to always use the double precision variant in the else-case, but we only generate float and double variants of the kernels (not integers), so didn't feel like making it more complicated than this.

rengolin accepted this revision.Nov 29 2022, 8:09 AM

LGTM, thanks!

MultiSource/Benchmarks/TSVC/tsc.inc
3778

UFF! This should really be CLOCKS_PER_SEC... But it's ok to make that a sweeping mechanical change on another patch.

This revision is now accepted and ready to land.Nov 29 2022, 8:09 AM
SjoerdMeijer added inline comments.Nov 29 2022, 8:28 AM
MultiSource/Benchmarks/TSVC/tsc.inc
3778

Wowsers!!! Good catch.
This is fine though if we divide by the same random number everywhere. ;-)

But while I am at it, I could do the find and replace as a follow up.

This revision was landed with ongoing or failed builds.Dec 5 2022, 1:11 AM
This revision was automatically updated to reflect the committed changes.