<div>Hi all,</div><div>š</div><div>Jan,</div><div>Its proper fix, IMHO. But may be its better to check for alignments we exactly know we support? I mean:</div><div>+ if (NumGPRs && (Align == 8 || Align == 16)) && ...</div><div>š</div><div>Otherwise you actually could just put the next line:</div><div>+ if (NumGPRs && (Align > 4)) && ...</div><div>š</div><div>Something stopped me from doing that last time. I need Renato and Chandler opinion.</div><div>What strategy should be used here? "Align > 4" or "(Align == 8 || Align == 16)"? Too small point details of course..</div><div>š</div><div>Comments in method mentioned only 8 bytes alignment:</div><div>ššš // Add padding for part of param recovered from GPRs, so<br />ššš // its last byte must be at address K*8 - 1.</div><div>That should be fixed with something like that:</div><div>-ššš // its last byte must be at address K*8 - 1.</div><div>+šš // For example, if Align == 8, its last byte must be at address K*8 - 1.</div><div>š</div><div>The same comments fix should be done in ARMMachineFunctionInfo.h, with "StByValParamsPadding" field.</div><div>š</div><div>-Stepan</div><div>š</div><div>04.02.2014, 02:52, "Renato Golin" <renato.golin@linaro.org>:</div><blockquote type="cite"><div><div><div>On 3 February 2014 21:17, Chandler Carruth <span><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br /><blockquote style="margin:0 0 0 0.8ex;border-left:1px #ccc solid;padding-left:1ex;"><div><div><div><span style="color:#222222;">FWIW, if possible it would be good to split this type of change into two patches -- the bug fix, and the change which uncovered it.</span></div></div></div></blockquote><div>š</div></div></div><div>Hi Chandler,</div><div>š</div><div>I don't think this is necessary. Not only the patch has 2 lines, but it's not uncommon for patches implementing a new feature to have the corresponding fixes, especially if it's only by introducing the feature that the "bug" shows up.</div><div>š</div><div>This case is even more obvious, since 8 was the maximum alignment and now it's 16. I think you got distracted by Mark's usage of the "bug" word. ;)</div><div>š</div><div>cheers,</div><div>--renato</div></div>,<p>_______________________________________________<br />llvm-commits mailing list<br /><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br /><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></p></blockquote>