<div dir="ltr"><div>Making such changes is not easy, but please try to find ways to minimize the number of ifdefs.</div><div>The more stuff you move to separate SystemZ-specific file(s) -- the better.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Apr 15, 2016 at 10:20 AM, Marcin Kościelnicki <span dir="ltr"><<a href="mailto:koriakin@0x04.net" target="_blank">koriakin@0x04.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 15/04/16 18:48, Kostya Serebryany wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I'd prefer if you could move this and similar pieces of code into<br>
separate SystemZ-specific file(s).<br>
The sanitizer run-time is already a hairy ball of #ifdefs, let's try not<br>
to make it worse.<br>
<br>
</blockquote>
<br></span>
Very well, but what exactly are you suggesting instead?<br>
<br>
s390 is just another architecture.  All the others already have lots of ifdefs in the code, I'm merely following the current style:<br>
<br>
projects/compiler-rt/lib $ grep -R __x86_64 *san* | wc -l<br>
79<br>
projects/compiler-rt/lib $ grep -R __powerpc *san* | wc -l<br>
69<br>
projects/compiler-rt/lib $ grep -R __aarch *san* | wc -l<br>
70<br>
projects/compiler-rt/lib $ grep -R __s390 *san* | wc -l<br>
60<br>
projects/compiler-rt/lib $ grep -R __i386 *san* | wc -l<br>
34<br>
projects/compiler-rt/lib $ grep -R __arm *san* | wc -l<br>
29<br>
projects/compiler-rt/lib $ grep -R __mips *san* | wc -l<br>
80<br>
<br>
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.<br>
<br>
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__).<br>
</blockquote></div><br></div>