[PATCH] D31947: [scudo] Android support groundwork

Kostya Kortchinsky via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 11 13:00:34 PDT 2017


cryptoad added inline comments.


================
Comment at: lib/scudo/scudo_allocator.cpp:176
     Chunk->loadHeader(&Header);
-    if (Header.State != ChunkQuarantine) {
+    if (UNLIKELY(Header.State != ChunkQuarantine)) {
       dieWithMessage("ERROR: invalid chunk state when recycling address %p\n",
----------------
alekseyshl wrote:
> Does UNLIKELY really improve the generated code?
I do like it as it will make the error condition the target of the conditional jump, and the rest of the code follows the regular flow.
For example for that particular check, on x64:
```
.text:0000000000053B72 E8 99 9A 00 00                    call    _ZN7__scudo20computeHardwareCRC32Ejm ; __scudo::computeHardwareCRC32(uint,ulong)
.text:0000000000053B77 4C 8B 44 24 08                    mov     r8, [rsp+78h+var_70]
.text:0000000000053B7C 66 41 39 C0                       cmp     r8w, ax
.text:0000000000053B80 0F 85 CA 02 00 00                 jnz     loc_53E50       ; error
.text:0000000000053B86 49 C1 EC 20                       shr     r12, 20h
```
The error is the target of the long jump.
I changed all the die conditions to be UNLIKELY as if they are hit, we terminate anyway.
The performance gain is not necessarily significant, but the assembly does look cleaner.


https://reviews.llvm.org/D31947





More information about the llvm-commits mailing list