This is an archive of the discontinued LLVM Phabricator instance.

llvm-reduce: Reduce ifuncs
ClosedPublic

Authored by arsenm on Nov 22 2022, 2:24 PM.

Details

Summary

Treat these basically the same way as aliases.

Diff Detail

Event Timeline

arsenm created this revision.Nov 22 2022, 2:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 22 2022, 2:24 PM
Herald added a subscriber: StephenFan. · View Herald Transcript
arsenm requested review of this revision.Nov 22 2022, 2:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 22 2022, 2:24 PM
Herald added a subscriber: wdng. · View Herald Transcript
aeubanks added inline comments.Nov 22 2022, 8:03 PM
llvm/test/tools/llvm-reduce/remove-ifunc.ll
63

mixing ptr and * will just end up using opaque pointers everywhere, so this part is pointless

arsenm added inline comments.Nov 23 2022, 7:11 AM
llvm/test/tools/llvm-reduce/remove-ifunc.ll
63

I'm constantly wondering if I should be trying to write code that bothers trying to work with typed pointers

llvm/tools/llvm-reduce/deltas/ReduceAliases.cpp
38

I'm wondering if it would be better to lower this by emitting a call to the resolver, plus the indirect call of the result

aeubanks added inline comments.Nov 23 2022, 9:28 AM
llvm/test/tools/llvm-reduce/remove-ifunc.ll
63

I wouldn't worry about it, typed pointers are deprecated and soon to be removed

llvm/tools/llvm-reduce/deltas/ReduceAliases.cpp
38

that does make more sense

arsenm updated this revision to Diff 478354.Nov 28 2022, 1:34 PM

Reduce to indirect call

precommit bot is saying the added tests are failing

llvm/lib/Transforms/Utils/ModuleUtils.cpp
292 ↗(On Diff #478354)

this deserves a unittest, then I'd say we don't need as many llvm-reduce-specific tests

llvm/test/tools/llvm-reduce/remove-ifunc-constantexpr.ll
2

not sure how useful an XFAIL test is if the verifier should fail on this anyway and I presume nobody's producing this sort of IR

should add a FIXME verifier test separately

arsenm added inline comments.Dec 1 2022, 4:51 PM
llvm/lib/Transforms/Utils/ModuleUtils.cpp
292 ↗(On Diff #478354)

I was going to introduce a separate lowering util pass later, I guess I can do that first

llvm/test/tools/llvm-reduce/remove-ifunc-constantexpr.ll
2

This is a leftover, D138634 is to disallow this

arsenm added inline comments.Dec 1 2022, 5:56 PM
llvm/lib/Transforms/Utils/ModuleUtils.cpp
292 ↗(On Diff #478354)

D139163 jumps ahead to a utility pass, and switching strategy to using a global constructor

arsenm updated this revision to Diff 484631.Dec 21 2022, 11:20 AM

Rebase on utility pass to use new constructor lowering

aeubanks accepted this revision.Jan 10 2023, 4:14 PM
This revision is now accepted and ready to land.Jan 10 2023, 4:14 PM
arsenm closed this revision.Jan 17 2023, 7:34 PM

84a413ad897c0f527095d04653dcd7333de8fb77