[compiler-rt] r266295 - [sanitizer] [SystemZ] Implement internal_mmap.

Marcin Koƛcielnicki via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 15 10:20:09 PDT 2016


On 15/04/16 18:48, Kostya Serebryany wrote:
> I'd prefer if you could move this and similar pieces of code into
> separate SystemZ-specific file(s).
> The sanitizer run-time is already a hairy ball of #ifdefs, let's try not
> to make it worse.
>

Very well, but what exactly are you suggesting instead?

s390 is just another architecture.  All the others already have lots of 
ifdefs in the code, I'm merely following the current style:

projects/compiler-rt/lib $ grep -R __x86_64 *san* | wc -l
79
projects/compiler-rt/lib $ grep -R __powerpc *san* | wc -l
69
projects/compiler-rt/lib $ grep -R __aarch *san* | wc -l
70
projects/compiler-rt/lib $ grep -R __s390 *san* | wc -l
60
projects/compiler-rt/lib $ grep -R __i386 *san* | wc -l
34
projects/compiler-rt/lib $ grep -R __arm *san* | wc -l
29
projects/compiler-rt/lib $ grep -R __mips *san* | wc -l
80

This is with quite a few unsubmitted s390 patches on top of what's 
already in review or svn.  s390 is certainly on the heavier side, but 
not out of the ordinary.

If you want to have arch-specific files for arch-specific functions, 
fine with me.  But that's not how things are currently done, and you'll 
still have a maze of #ifdefs left over - there's no sane way to expunge 
them from eg. D18888.  For things like D18895, you could split it off, 
but only at the cost of duplicating code.  You'll also still have 
#ifdefs for disabling the common versions of functions overriden by 
arch-specific ones (eg. internal_mmap will need #ifndef __s390__).


More information about the llvm-commits mailing list