[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3a24df992cf8: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum (authored by lenary). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

simon_tatham accepted this revision. simon_tatham added a comment. Thanks, that explanation looks fine. (And I agree that re-paragraphing it made more sense than my version) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119720/new/ https://reviews

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

dmgreen accepted this revision. dmgreen added a comment. This revision is now accepted and ready to land. Thanks. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119720/new/ https://reviews.llvm.org/D119720 _

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

lenary marked 3 inline comments as done. lenary added inline comments. Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:12-19 +// The intention is this: +// - Any 128-bit or 64-bit writes to the neon input register of an AES fused +// pair are safe (the inputs

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

lenary updated this revision to Diff 428978. lenary marked 3 inline comments as done. lenary added a comment. - Address comment nits - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119720/new/ https://reviews.llvm.org/D119720 Files: clang

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

dmgreen added inline comments. Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:146 + case ARM::VMVNd: + case ARM::VMVNq: + // VMOV of 64-bit value between D registers (when condition = al) lenary wrote: > dmgreen wrote: > > Perhaps add these,

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

lenary added a comment. Ack to all the comment clarifications, will update patch with those soon (probably tomorrow). Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:146 + case ARM::VMVNd: + case ARM::VMVNq: + // VMOV of 64-bit value between D registers (w

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

simon_tatham added inline comments. Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:12-19 +// The intention is this: +// - Any 128-bit or 64-bit writes to the neon input register of an AES fused +// pair are safe (the inputs are to the AESE/AESD instruction).

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

dmgreen added a comment. Looks OK to me, as far as I can see. If it worth adding a few extra instructions that may come up? Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:146 + case ARM::VMVNd: + case ARM::VMVNq: + // VMOV of 64-bit value between D regist

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

lenary updated this revision to Diff 421193. lenary marked 2 inline comments as done. lenary added a comment. - Updated set of safe instructions - Address reviewer feedback, including reordering passes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

lenary marked 6 inline comments as done. lenary added a comment. A few comments before I post the next version of the patch. Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:145 + case ARM::VMVNq: +return CondCodeIsAL(3); + // VMOV of 64-bit value between

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

dmgreen added inline comments. Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:200-207 + case ARM::VMOVv2f32: + case ARM::VMOVv4f32: + case ARM::VMOVv2i32: + case ARM::VMOVv4i32: + case ARM::VMOVv4i16: + case ARM::VMOVv8i16: + case ARM::VMOVv8i8:

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

lenary added a comment. Thanks for the review. Lots of comments inline, hopefully Phab doesn't mangle the large one. Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:145 + case ARM::VMVNq: +return CondCodeIsAL(3); + // VMOV of 64-bit value between D regi

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

dmgreen added inline comments. Comment at: llvm/lib/Target/ARM/ARMFixCortexA57AES1742098Pass.cpp:145 + case ARM::VMVNq: +return CondCodeIsAL(3); + // VMOV of 64-bit value between D registers (when condition = al) Can/should all these use findFirstPredOperan

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

lenary updated this revision to Diff 419208. lenary added a comment. - Remove whitespace change in ARMSubtarget - Remove commented-out debug lines in RDFGraph Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119720/new/ https://reviews.llvm.org/D11972

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

lenary planned changes to this revision. lenary added inline comments. Comment at: llvm/lib/CodeGen/RDFGraph.cpp:1096 RegisterRef RR = PDA.Addr->getRegRef(*this); -#ifndef NDEBUG -// Assert if the register is defined in two or more unrelated defs. -// This could happ

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

lenary removed a reviewer: kparzysz. lenary added a comment. @kparzysz I have rewritten this to avoid using RDFGraph, so I don't think this needs you to review it any more. Comment at: llvm/test/CodeGen/ARM/aes-erratum-fix.ll:49 + +define <16 x i8> @aese_once_via_val(<16 x i8>

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

lenary updated this revision to Diff 419202. lenary added a comment. - Rewrite pass in terms of ReachingDefAnalysis - Split tests into separate commit, for ease of review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119720/new/ https://reviews.ll

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

lenary added a comment. In D119720#3339855 , @dmgreen wrote: > I have a high level question regarding RDF, as I've not seen it used in many > other places, so it may be under-tested on Arm systems at the moment. This > currently, for all code, builds an

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

dmgreen added a comment. I have a high level question regarding RDF, as I've not seen it used in many other places, so it may be under-tested on Arm systems at the moment. This currently, for all code, builds an rdf graph, analyze the rdf graph for a fairly rare instructions, then fixes up the

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

lenary added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119720/new/ https://reviews.llvm.org/D119720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-

[PATCH] D119720: [ARM] Pass for Cortex-A57 and Cortex-A72 Fused AES Erratum

lenary added a comment. @kparzysz I've tagged you due to the changes in RDFGraph, which I believe you are the owner of. The asserts are hit in `llvm/test/CodeGen/ARM/inlineasm-error-t-toofewregs.ll` - the Register Allocator Chokes on the test due to not having enough registers for the inline a