This is an archive of the discontinued LLVM Phabricator instance.

[Polly] Support for math/misc intrinsics
ClosedPublic

Authored by jdoerfert on Sep 6 2014, 3:16 AM.

Diff Detail

Event Timeline

jdoerfert updated this revision to Diff 13364.Sep 6 2014, 3:16 AM
jdoerfert retitled this revision from to [Polly] Support for math/misc intrinsics.
jdoerfert updated this object.
jdoerfert edited the test plan for this revision. (Show Details)
jdoerfert added reviewers: grosser, sebpop, simbuerg.
jdoerfert added subscribers: Restricted Project, Unknown Object (MLST).
jdoerfert added a project: Restricted Project.
grosser requested changes to this revision.Sep 7 2014, 11:20 PM
grosser edited edge metadata.

Hi Johannes,

I just tried your test case without the actual change to Polly and it already passes. The reason is most likely that all these intrinsics are side effect free functions and can consequently already be handled by Polly.

I think adding this test case is still valuable, but I would suggest a couple of minor changes to minimize the test case:

  1. Call the function 'intrinsics' instead of jd
  2. Use just a single loop and a single array.

There is no value added by a second loop and a second array, it just adds more code to the test case.

  1. No need to call pow twice

We could possibly also remove the "intrinsics" comment (or explain that only more complex buildins such as memcpy need special handling).

Thanks,
Tobias

This revision now requires changes to proceed.Sep 7 2014, 11:20 PM

I will remove the last 3 lines from the test case, after that we need the patch. Or is it decided not to handle intrisics explicitly?

jdoerfert updated this revision to Diff 13382.Sep 8 2014, 3:03 AM
jdoerfert edited edge metadata.

[Polly] Support for math/misc intrinsics

Changed test case to use the new functionality.
jdoerfert updated this revision to Diff 13484.Sep 9 2014, 11:31 AM
jdoerfert edited edge metadata.

Removed explicit handling of math intrinsics and added testcases for misc intrinsics

In D5225#9, @jdoerfert wrote:

I will remove the last 3 lines from the test case, after that we need the patch. Or is it decided not to handle intrisics explicitly?

Hi Johannes,

I just looked back at this patch and it seems you are still proposing to handle intrinsics explicitly in the ScopDetection. There is a large number of target
specific intrinsics (e.g., test/CodeGen/X86/combine-avx2-intrinsics.ll), which do not affect memory and which we can currently be handled. What are your plans of handling those?

Cheers,
Tobias

grosser accepted this revision.Jan 24 2015, 1:48 PM
grosser edited edge metadata.

Uh, sorry. I was looking at an older version of this patch. The latest version looks in fact fine. Please feel free to commit.

Cheers,
Tobias

This revision is now accepted and ready to land.Jan 24 2015, 1:48 PM
jdoerfert closed this revision.Jan 25 2015, 11:46 AM

commited in 227054