Would you like me to commit for you?

In D123678#3538963, @phosek wrote:

Polly :: ScopDetect/dot-scops-npm.ll test has been flaking on our bots since this change landed with the following test error:

Script:
--
: 'RUN: at line 1';   /opt/s/w/ir/x/w/staging/llvm_build/bin/opt  -polly-process-unprofitable  -polly-remarks-minimal  -polly-use-llvm-names  -polly-import-jscop-dir=/opt/s/w/ir/x/w/llvm-llvm-project/polly/test/ScopDetect  -polly-codegen-verify  "-passes=polly-scop-printer" -disable-output < /opt/s/w/ir/x/w/llvm-llvm-project/polly/test/ScopDetect/dot-scops-npm.ll
: 'RUN: at line 2';   /opt/s/w/ir/x/w/staging/llvm_build/bin/FileCheck /opt/s/w/ir/x/w/llvm-llvm-project/polly/test/ScopDetect/dot-scops-npm.ll -input-file=scops.func.dot
--
Exit Code: 2

Command Output (stderr):
--
Writing 'scops.func.dot'...
FileCheck error: 'scops.func.dot' is empty.
FileCheck command line:  /opt/s/w/ir/x/w/staging/llvm_build/bin/FileCheck /opt/s/w/ir/x/w/llvm-llvm-project/polly/test/ScopDetect/dot-scops-npm.ll -input-file=scops.func.dot

--

Would it be possible to take a look?

Format codes

rebase to the master

Thanks.

In D124904#3504607, @Meinersbur wrote:

LGTM.

Do I have permission to commit for you?

Use pointer in the specialization of DOTGraphTraits

In D123678#3497332, @Meinersbur wrote:

LGTM. Thanks for your work! Do I have permission to commit for you?

Use GraphTraits<ScopDetection *>, remove redundant comments

Use pointer specialization by default, e.g. GraphTraits<ScopDetection *> but not GraphTraits<ScopDetection>

rebase to the latest main branch

Nice catch. Fixed in the latest commit.

move the default parameters to the declaration

In D123678#3490386, @Meinersbur wrote:

When compiling under Windows with cmake -DPOLLY_ENABLE_GPGPU_CODEGEN=ON, I get a linker error:

Polly.lib(PPCGCodeGeneration.obj) : error LNK2019: unresolved external symbol "class llvm::Pass * __cdecl polly::createDOTOnlyPrinterPass(void)" (?createDOTOnlyPrinterPass@polly@@YAPEAVPass@llvm@@XZ) referenced in function "public: __cdecl `anonymous namespace'::PollyForcePassLinki
ng::PollyForcePassLinking(void)" (??0PollyForcePassLinking@?A0x3c23cca7@@QEAA@XZ) [C:\Users\meinersbur\build\llvm-project\debug_vs17\tools\opt\opt.vcxproj]
Polly.lib(PPCGCodeGeneration.obj) : error LNK2019: unresolved external symbol "class llvm::Pass * __cdecl polly::createDOTOnlyViewerPass(void)" (?createDOTOnlyViewerPass@polly@@YAPEAVPass@llvm@@XZ) referenced in function "public: __cdecl `anonymous namespace'::PollyForcePassLinking
::PollyForcePassLinking(void)" (??0PollyForcePassLinking@?A0x3c23cca7@@QEAA@XZ) [C:\Users\meinersbur\build\llvm-project\debug_vs17\tools\opt\opt.vcxproj]
Polly.lib(PPCGCodeGeneration.obj) : error LNK2019: unresolved external symbol "class llvm::Pass * __cdecl polly::createDOTPrinterPass(void)" (?createDOTPrinterPass@polly@@YAPEAVPass@llvm@@XZ) referenced in function "public: __cdecl `anonymous namespace'::PollyForcePassLinking::Poll
yForcePassLinking(void)" (??0PollyForcePassLinking@?A0x3c23cca7@@QEAA@XZ) [C:\Users\meinersbur\build\llvm-project\debug_vs17\tools\opt\opt.vcxproj]
Polly.lib(PPCGCodeGeneration.obj) : error LNK2019: unresolved external symbol "class llvm::Pass * __cdecl polly::createDOTViewerPass(void)" (?createDOTViewerPass@polly@@YAPEAVPass@llvm@@XZ) referenced in function "public: __cdecl `anonymous namespace'::PollyForcePassLinking::PollyF
orcePassLinking(void)" (??0PollyForcePassLinking@?A0x3c23cca7@@QEAA@XZ) [C:\Users\meinersbur\build\llvm-project\debug_vs17\tools\opt\opt.vcxproj]
C:\Users\meinersbur\build\llvm-project\debug_vs17\Debug\bin\opt.exe : fatal error LNK1120: 4 unresolved externals [C:\Users\meinersbur\build\llvm-project\debug_vs17\tools\opt\opt.vcxproj]

I could not yet find a config that also fails under Linux, but it looks like some library dependencies are wrong.

format codes

format codes

fix and add test for the specialization

Fix wrong Twine usage

@Meinersbur Could you help me to commit this? 🍻

Remove duplicated implementation

Modify name from run*Impl to *GraphForFunction

Fixed in the newest diff.

Restore the pass of legacy pass manager.

Add runViewerImpl and runPrinterImpl to store the common implementation of the new and legacy pass.

DOTGraphTraitsPrinter is not used in this patch.

1. Modify the name from Legacy* to *LegacyPass
2. Upload the diff with git diff -U999999

Fix according to comments

As https://reviews.llvm.org/rGa8ac117d98f6548635b18b58159c9cfb31ba4762 is committed, I will close this revision.

Does anyone know why the unit test failed?

Oops. Sorry, it doesn't matter. The stack probing part will never be a parent frame of anything, so it would still be nice as it's the first frame to unwind. This patch is still good 🍻 .

Please correct me if I'm wrong. I realize that generating a DWARF message based on r11d doesn't help the unwinder to get a correct backtrace.

In D99585#2700778, @pengfei wrote:

Better to rebase it since many changes had committed with D99579.

rebase origin/main

Stack probing with loop should be discussed further in D99585.

In D99579#2662517, @nagisa wrote:

Landed this, thank you!

Do you have ability to commit this yourself, or would you need somebody to do it for you?

@nagisa This is another part of D98789, and is only implemented for 64-bit platform.

I installed clang-format later, and nothing found (there is no lint message). How could I update this patch and remove the "clang-format not found in user's PATH; not linting file." warning? (or just left it here 😋 )

@nagisa This is the inline part of D98789 😋 .

In D98789#2657046, @nagisa wrote:

@YangKeao Will you be pursuing this further? Should I take over this for you?

In D98789#2636879, @nagisa wrote:

btw I prototyped a D98906: [X86] Improve lowering of the unrolled inline-asm probing yesterday as an alternative approach towards improving the unrolled case.

left comments about 32bit

Remove dwarf information for 32bit and use R11 as the iterate bound / dwarf register

Normally, I'd expect some register is naturally free in the prologue, but you could get into weird situations. On 32-bit specifically, consider compiling with -mregparm=3; I think there are no registers which are unconditionally safe in that case. One possibility is to always use EAX, and just save/restore it if necessary. See isEAXAlive in X86FrameLowering::emitPrologue.

Alternatively, you could ensure that some callee-save GPR is spilled, and explicitly use that register. This is taking advantage of the fact this is part of the prologue: there can't be any other uses of callee-save registers at that point. (In theory, it might be possibly for an exotic calling convention to have no callee-save registers, but I don't think there are any in practice.)

A care must be taken to not overwrite the arguments as well. For instance on SysV x86_64 ABI rdi, rsi, rdx, rcx, r8, r9 are used to pass in integer arguments. For functions with a small number of arguments one of these could be reused, but if a function happens to use all of them, unconditional use of rdx would clobber the argument.

Use RAX/EAX as the iterate register, as RDX/EDX is used as arguments under systemv

Use RDX/EDX instead of RDI/EDI, as RDI/EDI is callee saved on Win64 calling convension

What registers can be used? I did a quick search and couldn't find anything.

reformat the code

• Use rdi to represent the stack bound and CFA
• Remove extra tailing offset adjust

In D98789#2632702, @efriedma wrote:

To clarify, I've done some more reading now, and figured out where I went wrong. For a long time, LLVM did not emit accurate unwind info to describe the prologue/epilogue (and still doesn't on some targets), so I was under the impression it wasn't possible. Clearly, it is, and it's implemented on x86.

---

The change to use r11 isn't implemented correctly: we can't adjust the stack pointer until *after* we've probed the relevant pages. It'll appear to work, but it won't actually provide complete protection if a signal handler triggers at the wrong time.

There has been a bug report for this on bugzilla. A more "downstream" context for this feature is discussed in rust#83139.