From 9efc4abd4bbdea025665c1df93b47b2716e3f703 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 3 Sep 2021 16:30:02 +0300 Subject: [PATCH 1/6] Allow instrumentation for methods with explicit tail calls --- src/coreclr/jit/fgbasic.cpp | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 6c65586fbca1c8..fe656cae2d0da5 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -2721,10 +2721,17 @@ unsigned Compiler::fgMakeBasicBlocks(const BYTE* codeAddr, IL_OFFSET codeSize, F if (fgCanSwitchToOptimized() && fgMayExplicitTailCall()) { - // Method has an explicit tail call that may run like a loop or may not be generated as a tail - // call in tier 0, switch to optimized to avoid spending too much time running slower code and - // to avoid stack overflow from recursion - fgSwitchToOptimized(); + if (!opts.jitFlags->IsSet(JitFlags::JIT_FLAG_BBINSTR) || + !((info.compFlags & CORINFO_FLG_DISABLE_TIER0_FOR_LOOPS) != 0)) + { + // Method has an explicit tail call that may run like a loop or may not be generated as a + // tail call in tier 0, switch to optimized to avoid spending too much time running slower + // code and to avoid stack overflow from recursion + + // However, let's not doing that if we're allowed to compile methods with loops to tier1 + // and going to instrument current method. + fgSwitchToOptimized(); + } } } else From 947544635748f78ac0b0d0c1e55011aa5fd6461c Mon Sep 17 00:00:00 2001 From: EgorBo Date: Fri, 3 Sep 2021 16:33:22 +0300 Subject: [PATCH 2/6] clean up --- src/coreclr/jit/fgbasic.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index fe656cae2d0da5..2ff26f651bff31 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -2722,14 +2722,14 @@ unsigned Compiler::fgMakeBasicBlocks(const BYTE* codeAddr, IL_OFFSET codeSize, F if (fgCanSwitchToOptimized() && fgMayExplicitTailCall()) { if (!opts.jitFlags->IsSet(JitFlags::JIT_FLAG_BBINSTR) || - !((info.compFlags & CORINFO_FLG_DISABLE_TIER0_FOR_LOOPS) != 0)) + ((info.compFlags & CORINFO_FLG_DISABLE_TIER0_FOR_LOOPS) != 0)) { // Method has an explicit tail call that may run like a loop or may not be generated as a // tail call in tier 0, switch to optimized to avoid spending too much time running slower // code and to avoid stack overflow from recursion - // However, let's not doing that if we're allowed to compile methods with loops to tier1 - // and going to instrument current method. + // However, let's not do it if we're allowed to compile methods with loops to tier1 + // and going to instrument the current method. fgSwitchToOptimized(); } } From 2b9ec2200477037c1834d3e579874422119f2e50 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Fri, 3 Sep 2021 16:37:23 +0300 Subject: [PATCH 3/6] Update fgbasic.cpp --- src/coreclr/jit/fgbasic.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 2ff26f651bff31..d1f9aed05094c8 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -2728,7 +2728,7 @@ unsigned Compiler::fgMakeBasicBlocks(const BYTE* codeAddr, IL_OFFSET codeSize, F // tail call in tier 0, switch to optimized to avoid spending too much time running slower // code and to avoid stack overflow from recursion - // However, let's not do it if we're allowed to compile methods with loops to tier1 + // However, let's not do it if we're allowed to compile methods with loops to tier0 // and going to instrument the current method. fgSwitchToOptimized(); } From b3697b0fd5163cf0c1894de120d4caf78c11f8bb Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Fri, 3 Sep 2021 17:09:32 +0300 Subject: [PATCH 4/6] Update fgbasic.cpp --- src/coreclr/jit/fgbasic.cpp | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index d1f9aed05094c8..152ef8887e9479 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -2719,19 +2719,13 @@ unsigned Compiler::fgMakeBasicBlocks(const BYTE* codeAddr, IL_OFFSET codeSize, F BADCODE3("tail call not followed by ret", " at offset %04X", (IL_OFFSET)(codeAddr - codeBegp)); } - if (fgCanSwitchToOptimized() && fgMayExplicitTailCall()) + if (fgCanSwitchToOptimized() && fgMayExplicitTailCall() && + ((info.compFlags & CORINFO_FLG_DISABLE_TIER0_FOR_LOOPS) != 0)) { - if (!opts.jitFlags->IsSet(JitFlags::JIT_FLAG_BBINSTR) || - ((info.compFlags & CORINFO_FLG_DISABLE_TIER0_FOR_LOOPS) != 0)) - { - // Method has an explicit tail call that may run like a loop or may not be generated as a - // tail call in tier 0, switch to optimized to avoid spending too much time running slower - // code and to avoid stack overflow from recursion - - // However, let's not do it if we're allowed to compile methods with loops to tier0 - // and going to instrument the current method. - fgSwitchToOptimized(); - } + // Method has an explicit tail call that may run like a loop or may not be generated as a + // tail call in tier 0, switch to optimized to avoid spending too much time running slower + // code + fgSwitchToOptimized(); } } else From 617e6e37a930e04c443411c47fa7612e98c6e5c3 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Fri, 3 Sep 2021 17:10:39 +0300 Subject: [PATCH 5/6] Update fgbasic.cpp --- src/coreclr/jit/fgbasic.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 152ef8887e9479..90691033b84875 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -2722,9 +2722,8 @@ unsigned Compiler::fgMakeBasicBlocks(const BYTE* codeAddr, IL_OFFSET codeSize, F if (fgCanSwitchToOptimized() && fgMayExplicitTailCall() && ((info.compFlags & CORINFO_FLG_DISABLE_TIER0_FOR_LOOPS) != 0)) { - // Method has an explicit tail call that may run like a loop or may not be generated as a - // tail call in tier 0, switch to optimized to avoid spending too much time running slower - // code + // Method has an explicit tail call that may run like a loop or may not be generated as a tail + // call in tier 0, switch to optimized to avoid spending too much time running slower code fgSwitchToOptimized(); } } From 247b6ebcbb167481573031d36adb8a8875c57630 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Mon, 6 Sep 2021 11:47:59 +0300 Subject: [PATCH 6/6] Address feedback --- src/coreclr/jit/fgbasic.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 90691033b84875..57c94e773ae1b0 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -2719,12 +2719,15 @@ unsigned Compiler::fgMakeBasicBlocks(const BYTE* codeAddr, IL_OFFSET codeSize, F BADCODE3("tail call not followed by ret", " at offset %04X", (IL_OFFSET)(codeAddr - codeBegp)); } - if (fgCanSwitchToOptimized() && fgMayExplicitTailCall() && - ((info.compFlags & CORINFO_FLG_DISABLE_TIER0_FOR_LOOPS) != 0)) + if (fgCanSwitchToOptimized() && fgMayExplicitTailCall()) { // Method has an explicit tail call that may run like a loop or may not be generated as a tail // call in tier 0, switch to optimized to avoid spending too much time running slower code - fgSwitchToOptimized(); + if (!opts.jitFlags->IsSet(JitFlags::JIT_FLAG_BBINSTR) || + ((info.compFlags & CORINFO_FLG_DISABLE_TIER0_FOR_LOOPS) != 0)) + { + fgSwitchToOptimized(); + } } } else