JIT: Remove BBF_FUNCLET_BEG#113975
Conversation
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
Files not reviewed (17)
- src/coreclr/jit/block.cpp: Language not supported
- src/coreclr/jit/block.h: Language not supported
- src/coreclr/jit/codegenarm.cpp: Language not supported
- src/coreclr/jit/codegenarm64.cpp: Language not supported
- src/coreclr/jit/codegencommon.cpp: Language not supported
- src/coreclr/jit/codegenlinear.cpp: Language not supported
- src/coreclr/jit/codegenloongarch64.cpp: Language not supported
- src/coreclr/jit/codegenriscv64.cpp: Language not supported
- src/coreclr/jit/codegenxarch.cpp: Language not supported
- src/coreclr/jit/compiler.h: Language not supported
- src/coreclr/jit/compiler.hpp: Language not supported
- src/coreclr/jit/emit.cpp: Language not supported
- src/coreclr/jit/fgbasic.cpp: Language not supported
- src/coreclr/jit/fgdiagnostic.cpp: Language not supported
- src/coreclr/jit/fgopt.cpp: Language not supported
- src/coreclr/jit/jiteh.cpp: Language not supported
- src/coreclr/jit/scopeinfo.cpp: Language not supported
ghost
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
@dotnet/jit-contrib PTAL. No diffs from the last run. I had to make a few changes that seem gratuitous to keep MinOpts TP from regressing. Thanks! |
src/coreclr/jit/codegenlinear.cpp
Outdated
| bool firstMapping = true; | ||
|
|
||
| if (block->HasFlag(BBF_FUNCLET_BEG)) | ||
| const bool isFuncletBeg = compiler->UsesFunclets() && compiler->bbIsHandlerBeg(block); |
There was a problem hiding this comment.
| const bool isFuncletBeg = compiler->UsesFunclets() && compiler->bbIsHandlerBeg(block); | |
| const bool isFuncletBeg = compiler->bbIsFuncletBeg(block); |
src/coreclr/jit/jiteh.cpp
Outdated
| // | ||
| bool Compiler::bbIsFuncletBeg(const BasicBlock* block) | ||
| { | ||
| if (UsesFunclets() && fgFuncletsCreated) |
There was a problem hiding this comment.
Should we even be asking this if fgFuncletsCreated is false?
There was a problem hiding this comment.
Perhaps rearrange so we assert(fgFuncletsCreated); under if(UsesFunclets())?
There was a problem hiding this comment.
When I started working on this, we had some paths where we would check for funclet start blocks before and after funclets were created, like in fgCanCompactBlock. I've since been able to remove these checks, so we can make the fgFuncletsCreated check an invariant.
|
/ba-g timeouts on Azure Linux 3 |
|
Still no diffs |
BBF_FUNCLET_BEG is set for the first block in each filter/handler region when we have funclets; this state can already be checked with Compiler::bbIsHandlerBeg. This PR replaces BBF_FUNCLET_BEG with a new helper method Compiler::bbIsFuncletBeg, which is identical in behavior to bbIsHandlerBeg when we have funclets, and (what should be) a const-evaluated return false when we don't use funclets at all. This was a no-diff change locally, though it does enable compaction of the first block in a handler region -- I don't see why such a transformation should be illegal.
2 participants
BBF_FUNCLET_BEGis set for the first block in each filter/handler region when we have funclets; this state can already be checked withCompiler::bbIsHandlerBeg. This PR replacesBBF_FUNCLET_BEGwith a new helper methodCompiler::bbIsFuncletBeg, which is identical in behavior tobbIsHandlerBegwhen we have funclets, and (what should be) a const-evaluatedreturn falsewhen we don't use funclets at all.This was a no-diff change locally, though it does enable compaction of the first block in a handler region -- I don't see why such a transformation should be illegal.