ScheduleDAGFast.cpp is compiled to object file, but the ScheduleDAGFast
object file isn't linked into clang executable file as no symbol is
referred by outside. Add calling to createXxx of ScheduleDAGFast.cpp,
then the ScheduleDAGFast object file will be linked into clang
executable file. The static RegisterScheduler will register scheduler
fast and linearize at clang boot time.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
clang/test/CodeGen/pre-ra-sched.c | ||
---|---|---|
1 | Should we add test under llvm/test? A bit strange that changing LLVM code while adding test in clang/test. | |
llvm/include/llvm/CodeGen/TargetLowering.h | ||
105 | This comma should be removed. | |
llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp | ||
273–276 | I saw they are always registered in ScheduleDAGFast.cpp: |
Thanks Pengfei for your review comments!
llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp | ||
---|---|---|
273–276 | Please also help to have a review of the Summary. It's a little bit of a trick. ScheduleDAGFast.cpp is compiled to object file, but the object file isn't linked into clang executable file as no symbol is referred by outside without this patch. |
llvm/test/CodeGen/Generic/pre-ra-sched.c | ||
---|---|---|
1 ↗ | (On Diff #343598) | You can't run clang from an llvm test directory. There's no guarantee clang will have been compiled. |
llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp | ||
---|---|---|
273–276 | That wasn't very clear from your summary. The "the object file isn't linked into clang executable file as no symbol is referred by outside without this patch" in this comment was much more help. Can you put something like that in summary? Why are they being stripped from clang but not llc? |
llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp | ||
---|---|---|
273–276 | Oh I see, llc uses include/llvm/CodeGen/LinkAllCodegenComponents.h to force these to link in. |
llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp | ||
---|---|---|
273–276 | Thanks! I modified summary. could you please have a review? | |
llvm/test/CodeGen/Generic/pre-ra-sched.c | ||
1 ↗ | (On Diff #343598) | Thanks for notifying no guarantee clang in llvm test directory! I moved the test back to clang/test/CodeGen/. |
I see. LGTM, but let's wait one or more days to see if others object it.
llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp | ||
---|---|---|
275–276 | Can this be omitted given createFastDAGScheduler should make it linked. |
llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp | ||
---|---|---|
275–276 | For making scheduler fast and linearize visible by clang -pre-RA-sched, yes, createDAGLinearizer can be omitted. |
clang/test/CodeGen/pre-ra-sched.c | ||
---|---|---|
2–3 | The test as was committed is bogus. It checks the output binary(!) stream for an error message when stderr was not even redirected. Please fix. |
clang/test/CodeGen/pre-ra-sched.c | ||
---|---|---|
2–3 |
clang/test/CodeGen/pre-ra-sched.c | ||
---|---|---|
2–3 | Thanks for your review comment and fixing! |
clang/test/CodeGen/pre-ra-sched.c | ||
---|---|---|
2–3 | No problem. Thanks for looking into it. |
Should we add test under llvm/test? A bit strange that changing LLVM code while adding test in clang/test.