[PATCH] D34409: Use 64bit jump table with large code model on 64bit

Yichao Yu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 20 14:20:52 PDT 2017


yuyichao added a comment.

> I expect something better could be done for PPC, but this is entirely in line with the existing 32-bit code and correctness comes before performance. I pretty strongly object to characterising the patch as "wrong".

Exactly, the code generated for aarch64, ppc64 and x86-64 in this case was obviously wrong in correctness sense (feels wierd to say it out...). The code generated for x86-64 and aarch64 looks correct to me. I'm not very familiar with ppc64 assembly to tell but given the code generating the assembly is pretty generic I expect it to be correct at least. I have no idea what exactly would be a better code to generate for either of these cases. I do agree a function can be assumed to be smaller than 2G so if a different but existing code path can be used I'll be happy to make that change for a specific arch. Otherwise, I'd like to keep the "generic" way currently used since that matches what's done in 32bit mode and leave the performance optimization part to people more familiar with the performance model and trade offs on different archs.

FWIW, the PPC backend does seems to be doing some transformation so that a function local offset is end up being used. The original function never returns `EK_GPRel32BlockAddress` and I can't really verify the correctlyness of the resulting assembly so I went with the version that shares the code path with two other archs that I can verify. If `EK_GPRel32BlockAddress` should work on ppc64 and generates an pc offset table, I'll be happy to make that change.


https://reviews.llvm.org/D34409





More information about the llvm-commits mailing list