In D102592#2769811, @eugenis wrote:

I think it is too hit-and-miss to add even under a flag. It's just not the right approach - but I also don't know what the right approach would be.

Perhaps adding a small left redzone for all globals and reducing the right redzone to keep the total size under control? This way when most globals are instrumented we get approximately the same amount of redzone between any 2 of them, but also a little on the left of the first one.

Apply optimization when SizeInBytes <= MinRZ/2

In D102592#2764357, @eugenis wrote:

You are adding a new global to every translation unit.

• Private linkage would not allow them to be merged, this can have significant binary size and RAM overhead.

In D102592#2764085, @vitalybuka wrote:

Would you mind to add compiler-rt/test/asan which triggers this one?

Update as suggected

Fix typos

Fix all tests

Fix some tests

Fix some tests

Fix clang/codegen tests

Fix all tests

Fix tests

In D99404#2657112, @MaskRay wrote:

Perhaps cross check vscode extensions.

Update

1. Drop Emacs 23 support, since it's 10 years old.
2. Highlight more keywords

In D98659#2633657, @lkail wrote:

In D98659#2632859, @yshui wrote:

Since this patch already changed output of many test cases, do we still need a test case specifically for this patch?

You'd better add some mir tests to demonstrate the idea, w/o -g.

Move test to machine-copy-props.mir

Please do move https://reviews.llvm.org/D98401 regression test to this patch set

In D98401#2627512, @yshui wrote:

https://reviews.llvm.org/D98659 could supersede this

Update test

CHECK-NEXT directive should be used

The debuginfo should be udpate. It will be implemented in the next patch update.

Add a regression test

。。。

Sorry

In D98401#2620620, @fhahn wrote:

You can look at some of the existing tests for machine-cp as inspiration (grep for machine-cp in llvm/test/Codegen) ,e.g. see https://github.com/llvm/llvm-project/blob/main/llvm/test/CodeGen/AArch64/machine-cp-clobbers.mir

For more background about Machine IR & using it for testing, please take a look at https://llvm.org/docs/MIRLangRef.html#mir-testing-guide

In D98401#2619176, @yshui wrote:

Because you replaced $rbp with $rbx, surely you would have to replace it everywhere. Removing the DBG_VALUE is potential information loss.

In D98401#2619087, @yshui wrote:

renamable $rbx = nuw LEA64r %stack.5.agg.tmp.i, 1, $noreg, 8, $noreg
DBG_VALUE $rbp, $noreg, !"this", !DIExpression(), debug-location !293; hot.cpp:0 @[ hot.cpp:84:39 @[ hot.cpp:108:11 ] ] line no:0

Shouldn't the $rbp in DBG_VALUE be updated as well?

In D98401#2618987, @fhahn wrote:

please add a unit test

The tests failed though