xen0n added inline comments.

================
Comment at: compiler-rt/lib/lsan/lsan_common.cpp:281
+#  elif defined(__loongarch_lp64)
+  return ((p >> 47) == 0);
 #  else
----------------
tangyouling wrote:
> xen0n wrote:
> > Since our VM layout is actually flexible, would it be better to document 
> > this, like "Support only the most common VM layout on LoongArch that allows 
> > 47 bits of user-space VMA"? Exact wording could be optimized, I'm only 
> > describing the gist here.
> > Since our VM layout is actually flexible, would it be better to document 
> > this, like "Support only the most common VM layout on LoongArch that allows 
> > 47 bits of user-space VMA"? Exact wording could be optimized, I'm only 
> > describing the gist here.
> 
> How about "Allow 47-bit user-space VMA at current"?
"at present"; if you plan to revisit this later for properly supporting 
variable VM layouts, omitting the justification is fine to me.


================
Comment at: compiler-rt/test/asan/TestCases/Linux/leak_check_segv.cpp:4
 // REQUIRES: leak-detection
-#include <stdlib.h>
+#include <sanitizer/lsan_interface.h>
 #include <stdio.h>
----------------
tangyouling wrote:
> xen0n wrote:
> > Why unnecessarily reorder things, especially putting this //in front of// 
> > the standard library includes?
> > Why unnecessarily reorder things, especially putting this //in front of// 
> > the standard library includes?
> 
> An automatic adjustment of the format.
So you mean you're actually unable to avoid this diff damage?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139686

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

Reply via email to