The commit following commit adds code that causes valgrind to crash on a few of the ppc64 specific tests: none/tests/ppc64/test_isa_2_06_part3 (stdout) none/tests/ppc64/test_isa_2_06_part3 (stderr) none/tests/ppc64/test_isa_2_07_part2 (stdout) none/tests/ppc64/test_isa_2_07_part2 (stderr) commit 076a79a48e251067758e1e9d8e50681450ed3889 Author: Julian Seward <jseward@acm.org> Date: Wed Nov 27 08:52:45 2019 +0100 'grail' fixes for ppc32 and ppc64: * do_minimal_initial_iropt_BB: for ppc64, flatten rather than assert flatness. (Kludge. Sigh.) etc. The patch adds the following code in ir_opt.c // FIXME2 The TOC-redirect-hacks generators in m_translate.c -- gen_PUSH() // and gen_PO() -- don't generate flat IR, and so cause this assertion // to fail. For the time being, hack around this by flattening, // rather than asserting for flatness, on the afflicted platforms. // This is a kludge, yes. if (guest_arch == VexArchPPC64) { bb0 = flatten_BB(bb0); // Kludge! } else { vassert(isFlatIRSB(bb0)); // How it Really Should Be (tm). } The issue comes from the new expressions generated by flatten_BB(bb0). The flatten_BB() generates V128 expressions for Iex_ITE which is not supported.
Created attachment 126876 [details] Additional PPC 64 grail fixes The issue that is causing regression errors on PPC64 has to do with the grail changes as mentioned in some private emails. Specifically the commit in question is: commit 076a79a48e251067758e1e9d8e50681450ed3889 Author: Julian Seward <jseward@acm.org> Date: Wed Nov 27 08:52:45 2019 +0100 'grail' fixes for ppc32 and ppc64: * do_minimal_initial_iropt_BB: for ppc64, flatten rather than assert flatness. (Kludge. Sigh.) etc. The patch adds the following code in ir_opt.c // FIXME2 The TOC-redirect-hacks generators in m_translate.c -- gen_PUSH() // and gen_PO() -- don't generate flat IR, and so cause this assertion // to fail. For the time being, hack around this by flattening, // rather than asserting for flatness, on the afflicted platforms. // This is a kludge, yes. if (guest_arch == VexArchPPC64) { bb0 = flatten_BB(bb0); // Kludge! } else { vassert(isFlatIRSB(bb0)); // How it Really Should Be (tm). } The issue comes from the new expressions generated by flatten_BB(bb0). As mentioned in previous private emails, the flatten_BB() generates V128 expressions for Iex_ITE which is not supported. The following patch adds the needed support for Iex_ITE for V128 expressions. I kinda get what the Iex_ITE needs to do but don't claim to completely understand it all or why the kludge calls flatten_BB() only for the PPC64 architecture.
Created attachment 126880 [details] Updated Additional PPC 64 grail fixes Per the path review, Pin_V128CMov and Pin_AvCMov are functionally the same. No need to add Pin_V128CMov. Updated the patch and retested and fixes three tests as expected.
(In reply to Carl Love from comment #2) > Created attachment 126880 [details] > Updated Additional PPC 64 grail fixes > > Per the path review, Pin_V128CMov and Pin_AvCMov are functionally the same. > No need to add Pin_V128CMov. Updated the patch and retested and fixes three > tests as expected. That looks fine. Please land!
Patch committed. commit c4c289ae994dfb8195e8b4889746d5e3d8296d25 (HEAD -> master, origin/master, origin/HEAD) Author: Carl Love <carll@us.ibm.com> Date: Wed Mar 18 12:29:20 2020 -0500 additional grail' fixes for ppc32 and ppc64 The grail changes introduce a kludge call for ppc64. The call fails on some tests as the flatten call generates adds addStmtToIRSB(bb, IRStmt_WrTmp(t1, IRExpr_ITE(flatten_Expr(bb, ex->Iex.ITE.cond), flatten_Expr(bb, ex->Iex.ITE.iftrue), flatten_Expr(bb, ex->Iex.ITE.iffalse)))); for V128 expressions. Iex_ITE isn't supported for V128 type. This patch adds the needed V128 support for the Iex_ITE expressions. Bugzilla 418004
Closing....