Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GSoC] Noinline attribute when using llvm-print-ir #6280

Merged
merged 3 commits into from Jun 27, 2017
Merged

[GSoC] Noinline attribute when using llvm-print-ir #6280

merged 3 commits into from Jun 27, 2017

Conversation

coodie
Copy link
Contributor

@coodie coodie commented May 20, 2017

In full optimization stage function sometimes was inlined and removed from llvm module, easiest way to fix this is to add 'noinline' attribute for it.

The preferred way however is to add noinline pragma before function we want to print so that this option doesn't interfere with other optimization options and it is explicitly stated in code.

Other solution would be add llvm-print-inline option which would specify if we want to turn this off or on.

Anyway, now we can see that adding noinline attribute is very easy to do.

@@ -56,6 +54,10 @@
#include <inttypes.h>
#include <stdint.h>

#ifdef HAVE_LLVM
#include "llvm/IR/Attributes.h"
#endif
Copy link
Contributor Author

@coodie coodie May 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like putting these ifdefs here, but I can't see the default place where to put these ifdefs. Any guidance would be great.

Copy link
Member

@mppf mppf May 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is OK. I'd probably put the LLVM include before the first system include (<algorithm>). compiler/include/llvmUtil.h might work too but it's probably smarter to just put the include you need for this file here. Speaking of that, I'm surprised that the other llvm:: stuff in this file worked and addFnAttr didn't. Are you sure you can't just remove the new include entirely?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, apparently it does, I can recall that it didn't compile without this header but apparently my crazy mind made it up.

@mppf
Copy link
Member

mppf commented Jun 23, 2017

Passed full local testing.

  • full local CHPL_LLVM=llvm testing
  • full local --llvm testing

@mppf mppf merged commit c7fc6a5 into chapel-lang:master Jun 27, 2017
@coodie coodie changed the title Noinline attribute when using llvm-print-ir [GSoC] Noinline attribute when using llvm-print-ir Aug 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants