This is an archive of the discontinued LLVM Phabricator instance.

Unit test for Conditionally eliminate library calls
Needs ReviewPublic

Authored by xur on Sep 9 2016, 11:55 AM.

Details

Summary

This patch is the unit test for conditionally eliminate library calls patch,
which is being reviewed here:
https://reviews.llvm.org/D24414

Diff Detail

Event Timeline

xur updated this revision to Diff 70882.Sep 9 2016, 11:55 AM
xur retitled this revision from to Unit test for Conditionally eliminate library calls.
xur updated this object.
xur added reviewers: davidxl, hfinkel, mehdi_amini.
xur added subscribers: llvm-commits, xur.
davidxl added inline comments.Sep 28 2016, 11:54 AM
SingleSource/Regression/C++/cdce_macro.inc
16 ↗(On Diff #70882)

use upper case for macro?

Why is the macro needed?

20 ↗(On Diff #70882)

why 10 ?

21 ↗(On Diff #70882)

-->TEST_VAL_RANGE?

33 ↗(On Diff #70882)

-->TEST_VAL

This macro be used in TEST_VAL_RANGE definition

33 ↗(On Diff #70882)

make it take a value argument

xur marked 2 inline comments as done.Oct 11 2016, 4:03 PM
xur added inline comments.
SingleSource/Regression/C++/cdce_macro.inc
16 ↗(On Diff #70882)

OK. used the upper case in the new patch.

We need this because, we need to postfix "l" or "f" to the name for long double and float. For double type, we don't need to do anything, that's the default here.

20 ↗(On Diff #70882)

just an arbitrary number. Do you want it to be a larger number? testing all the integers seems to be excessive.

xur updated this revision to Diff 74308.Oct 11 2016, 4:08 PM

Integrated David's review comments and changed the file names in sync to https://reviews.llvm.org/D24414