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

llvm-cpufeatures: get TargetMachine from the MachineModuleInfoWrapperPass pass #44005

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Feb 1, 2022

This is usually only supposed to be accessible to a MachineModulePass,
but we can trick llvm to give us access to the TargetMachine here too.

Also hack in a sysimg check also, so that the compile result is ensured
to be compatible with the loaded image too (we disable loading of the
sysimg when emitting a new compile).

…Pass pass

This is usually only supposed to be accessible to a MachineModulePass,
but we can trick llvm to give us access to the TargetMachine here too.

Also hack in a sysimg check also, so that the compile result is ensured
to be compatible with the loaded image too (we disable loading of the
sysimg when emitting a new compile).
return PreservedAnalyses::all();
}

extern "C" JL_DLLEXPORT ::llvm::PassPluginLibraryInfo
llvmGetPassPluginInfo() {
Copy link
Member

Choose a reason for hiding this comment

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

We should move that out to a different file, since we will need to add all the other NewPM to this as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

You have all the other NewPM changes, right? So I will leave this up to you to move.

Copy link
Member

Choose a reason for hiding this comment

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

There are already other passes that support the NewPM, but sure I can do that.

@@ -0,0 +1,24 @@
; RUNx: opt --mtriple=`llvm-config --host-target` -enable-new-pm=1 --load-pass-plugin=libjulia-codegen%shlibext -passes='require<machine-module>,CPUFeatures' -S %s | FileCheck %s --check-prefixes=CHECK,CHECK-any
Copy link
Member

Choose a reason for hiding this comment

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

require<machine-module> needs a LLVm patch right?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, right now it this is disabled, with the x

@vchuravy vchuravy requested a review from maleadt February 1, 2022 20:35
@vchuravy vchuravy added this to the 1.8 milestone Feb 1, 2022
@vchuravy
Copy link
Member

vchuravy commented Feb 1, 2022

x-ref #43085 (comment) as context

@Keno
Copy link
Member

Keno commented Feb 1, 2022

How does this interact with multiversioning? Shouldn't we be looking at the features that are annotated in the function attribute rather than the target machine?

@vtjnash
Copy link
Member Author

vtjnash commented Feb 1, 2022

This is the fallback code, when there are no target-features set.

@gbaraldi
Copy link
Member

gbaraldi commented Feb 2, 2022

Should the same approach be used for the Float16 PR?

@JeffBezanson JeffBezanson removed this from the 1.8 milestone May 17, 2022
@maleadt maleadt removed their request for review May 18, 2022 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants