This is an archive of the discontinued LLVM Phabricator instance.

[DebugInfo] Restore test case for long double constants.
ClosedPublic

Authored by dgross on Dec 8 2016, 3:47 PM.

Details

Summary

D27549 (partial fix for PR26619) emits a constant value in the debug
metadata for a floating-point static const that does not exceed 64
bits in size. Whether or not a long double exceeds 64 bits in size
depends on the target. Modify the test case so that it expects a
constant value for long double if and only if the long double is no
larger than 64 bits.

Diff Detail

Repository
rL LLVM

Event Timeline

dgross updated this revision to Diff 80840.Dec 8 2016, 3:47 PM
dgross retitled this revision from to [DebugInfo] Restore test case for long double constants..
dgross updated this object.
dgross added reviewers: cfe-commits, probinson.
dgross added a comment.Dec 8 2016, 3:51 PM

I don't know exactly what the RUN syntax supported by lit is. What I've done here looks complex, but it does work for Linux. What about other platforms?

Is there some better way of writing a test case where the checks to be done by FileCheck depend on some property of the file being analyzed (here, the size of long double, as embedded in the debug metadata)? I don't see any support for conditions in FileCheck.

Maybe I'm going about this completely the wrong way?

probinson edited edge metadata.Dec 13 2016, 3:06 PM

As dblaikie said in email, probably better to make this X86-specific; if long-double varies by OS you can put in a specific triple.

FTR we don't rely on Perl being available everywhere, anything that does this kind of scripty stuff uses Python.

test/CodeGen/debug-info-static-const-fp.c
1 ↗(On Diff #80840)

Sorry didn't notice this before, %clang is for driver tests only; use %clang_cc1 here.

As dblaikie said in email, probably better to make this X86-specific; if long-double varies by OS you can put in a specific triple.

I think with this approach I'd want two test cases that are identical except for consideration of long double size: One that specifies a target where long double is "small" and one where it is "large". Actually, I guess I could make this a single test case, but have two compile and two filecheck lines, specifying different triples, as you suggest. Or even three: One for whatever the default target is (doesn't check long double), one for a triple where long double is small, and one for a triple where long double is large.

FTR we don't rely on Perl being available everywhere, anything that does this kind of scripty stuff uses Python.

So would a Python equivalent of the Perl be acceptable? I think this is an academic question -- better to explicitly test multiple triples.

dgross updated this revision to Diff 81321.Dec 13 2016, 3:57 PM
dgross edited edge metadata.

Incorporate code review comments.

  • Use %clang_cc1 not %clang
  • Rather than trying to determine long double size for target, compile and check multiple times, and only check behavior of long double for known targets
dgross marked an inline comment as done.Dec 13 2016, 4:05 PM
probinson accepted this revision.Dec 13 2016, 4:50 PM
probinson edited edge metadata.

So would a Python equivalent of the Perl be acceptable? I think this is an academic question -- better to explicitly test multiple triples.

There are a small number of tests that run Python scripts, but they are generally doing something really unusual (I admit I haven't gone to look, but that's my recollection). If you can get proper coverage without Python, and it doesn't require standing on your head, people generally prefer that. In this case, multiple RUNs with different triples will do the job, so that's preferable.

LGTM.

This revision is now accepted and ready to land.Dec 13 2016, 4:50 PM
This revision was automatically updated to reflect the committed changes.