Page MenuHomePhabricator

[Bindings] Add LLVMAddInstructionSimplifyPass
ClosedPublic

Authored by aeubanks on Aug 28 2020, 12:35 AM.

Diff Detail

Event Timeline

aeubanks created this revision.Aug 28 2020, 12:35 AM
Herald added a project: Restricted Project. · View Herald Transcript
aeubanks requested review of this revision.Aug 28 2020, 12:35 AM

Looks great to me, that'll certainly fit the bill for mesa (not sure though on the name should it have the Legacy in the name too for consistency - either way looks fine to me however).

hans added a subscriber: hans.Aug 28 2020, 1:14 AM

Looks great to me, that'll certainly fit the bill for mesa (not sure though on the name should it have the Legacy in the name too for consistency - either way looks fine to me however).

It would be great to also have this mentioned in the release notes, to make it easier for other llvm-as-a-library users to tell what they have to do.

aeubanks updated this revision to Diff 288640.Aug 28 2020, 9:35 AM

release notes

Looks great to me, that'll certainly fit the bill for mesa (not sure though on the name should it have the Legacy in the name too for consistency - either way looks fine to me however).

The name should be consistent with the other C binding names, "legacy" is an implementation detail.

It would be great to also have this mentioned in the release notes, to make it easier for other llvm-as-a-library users to tell what they have to do.

Done.

sroland accepted this revision.Tue, Sep 1, 10:44 AM
This revision is now accepted and ready to land.Tue, Sep 1, 10:44 AM
This revision was landed with ongoing or failed builds.Tue, Sep 1, 12:39 PM
This revision was automatically updated to reflect the committed changes.

Actually it doesn't seem to work, I get undefined references to LLVMAddInstructionSimplifyPass at link time in mesa.
Not entirely sure why (would InstSimplifyPass.cpp have to include Scalar.h?), but moving the LLVMAddInstructionSimplifyPass() function to Scalar.cpp (all the other scalar passes have the wrapper function there already) fixes it.

Actually it doesn't seem to work, I get undefined references to LLVMAddInstructionSimplifyPass at link time in mesa.
Not entirely sure why (would InstSimplifyPass.cpp have to include Scalar.h?), but moving the LLVMAddInstructionSimplifyPass() function to Scalar.cpp (all the other scalar passes have the wrapper function there already) fixes it.

Sorry about that, https://reviews.llvm.org/D87041.