The LLINT loadisFromInstruction macro aims to load the least significant 32-bit word from the 64-bit bytecode instruction stream and sign extend it. For big endian machines, the current implementation would load the wrong 32-bit word.
Created attachment 229055 [details] The patch.
Created attachment 229056 [details] patch 2: updated comment.
Comment on attachment 229056 [details] patch 2: updated comment. Does this fix all big endian issues? It seems to me like we do lots of loadi instructions assuming little endianness.
(In reply to comment #3) > (From update of attachment 229056 [details]) > Does this fix all big endian issues? It seems to me like we do lots of loadi instructions assuming little endianness. I would not presume that. Tomas has been analyzing big endian failures in https://bugs.webkit.org/show_bug.cgi?id=12874. I’m just knocking them down one at a time when he reports them. This one happens to be the next one. It may or may not be the last.
(In reply to comment #4) > (In reply to comment #3) > > (From update of attachment 229056 [details] [details]) > > Does this fix all big endian issues? It seems to me like we do lots of loadi instructions assuming little endianness. > > I would not presume that. Tomas has been analyzing big endian failures in https://bugs.webkit.org/show_bug.cgi?id=12874. I’m just knocking them down one at a time when he reports them. This one happens to be the next one. It may or may not be the last. Hmm. Assuming this isn't the last one, is there a better way to handle these bugs rather than slapping if BIG_ENDIAN everywhere? Could this be handled in the backend somehow?
(In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > (From update of attachment 229056 [details] [details] [details]) > > > Does this fix all big endian issues? It seems to me like we do lots of loadi instructions assuming little endianness. > > > > I would not presume that. Tomas has been analyzing big endian failures in https://bugs.webkit.org/show_bug.cgi?id=12874. I’m just knocking them down one at a time when he reports them. This one happens to be the next one. It may or may not be the last. > > Hmm. Assuming this isn't the last one, is there a better way to handle these bugs rather than slapping if BIG_ENDIAN everywhere? Could this be handled in the backend somehow? Or maybe we just need a macro that provides a good abstraction for loading 32 bits that has a single if BIG_ENDIAN and then use that everywhere.
(In reply to comment #5) > (In reply to comment #4) > > (In reply to comment #3) > > > (From update of attachment 229056 [details] [details] [details]) > > > Does this fix all big endian issues? It seems to me like we do lots of loadi instructions assuming little endianness. > > > > I would not presume that. Tomas has been analyzing big endian failures in https://bugs.webkit.org/show_bug.cgi?id=12874. I’m just knocking them down one at a time when he reports them. This one happens to be the next one. It may or may not be the last. > > Hmm. Assuming this isn't the last one, is there a better way to handle these bugs rather than slapping if BIG_ENDIAN everywhere? Could this be handled in the backend somehow? Correction on the url from my previous comment: it should be https://bugs.webkit.org/show_bug.cgi?id=128743. That would be inappropriate to rig loadi in the backend because a loadi can be used to load the most significant 32-bit if that is the intent. I’m going by the rule that the LLINT asm is assembly, and we should coding it the same way an assembly programmer would code. In this case, the issue isolated to a macro, and that “if BIG_ENDIAN” is isolated. Note: this is not a new idiom. It was already in use previously, and I adopted it here to solve the issue.
Comment on attachment 229056 [details] patch 2: updated comment. Okay, we'll refactor as we encounter new issues. YAGNI! r=me
Comment on attachment 229056 [details] patch 2: updated comment. Clearing flags on attachment: 229056 Committed r167076: <http://trac.webkit.org/changeset/167076>
All reviewed patches have been landed. Closing bug.
For the record, this fix was invalid. https://bugs.webkit.org/show_bug.cgi?id=132330 will undo it, and make it right.