On Tue, 30 Jul 2019 at 21:00, Aleksandar Markovic <aleksandar.m.m...@gmail.com> wrote: > > On Thu, Jul 25, 2019 at 3:25 AM <tony.ngu...@bt.com> wrote: > > > Rename ALIGNED_ONLY to TARGET_ALIGNED_ONLY for clarity and move > > defines out of target/foo/cpu.h into configure, as we do with > > TARGET_WORDS_BIGENDIAN, so that it is always defined early. > > > > Also, poison the symbol in include/exec/poison.h to prevent use in > > common code. > > > > > Hi, Tony. > > Thanks for this new version. > > When one mentions "also" in the commit message, this is a kind of strong > indication that the patch should be split into two patches. > > So, could you please consider moving "poison" part into a separate patch?
I think in this case the issue is more in the phrasing of the commit message rather than the patch composition. The patch is introducing TARGET_ALIGNED_ONLY as something defined by configure, and the correct status for that macro is "needs to be in poison.h"; having an intermediate state where the macro exists but isn't poisoned isn't really a logical split of the work. (The previous ALIGNED_ONLY didn't have so much need to be poisoned because it was defined in cpu.h and so the only way to expect to get it was by pulling in cpu.h.) thanks -- PMM
