russell.gallop added a comment.

In D86694#2288660 <https://reviews.llvm.org/D86694#2288660>, @aganea wrote:

> I'm also in favor, I think this is good direction ahead. It'd be nice if 
> following issues were fixed -- in subsequent patches if you wish:
>
> - Stage1 `ninja check-scudo` fails many tests for me, see F13037612: 
> errors.txt <https://reviews.llvm.org/F13037612>.

I intend to fix these (or mark an unsupported) for this patch.

> - Stage2 `ninja check-llvm` fails after a while on high-core machines (4TB 
> issue mentionned in comments above). Lowering `AllocatorSize` to 256GB would 
> fix the issue on the short term.

I'll add that.

> - Fix & test the "exclusive" mode (or just skip to scudo-standalone if it's 
> too complicated).

I don't intend to do that for this patch, as shared mode works. I will 
re-evaluate after the above.

I also intend to remove the -fsanitize related changes. I believe that they're 
not required on Windows.

Thanks
Russ



================
Comment at: compiler-rt/lib/scudo/scudo_platform.h:67
 #if SANITIZER_CAN_USE_ALLOCATOR64
 # if defined(__aarch64__) && SANITIZER_ANDROID
 const uptr AllocatorSize = 0x4000000000ULL;  // 256G.
----------------
aganea wrote:
> `&& SANITIZER_WINDOWS` ?
I think that should be:


```
# if (defined(__aarch64__) && SANITIZER_ANDROID) || SANITIZER_WINDOWS
```

I'll either do that or add a new elif.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86694/new/

https://reviews.llvm.org/D86694

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to