> > Hi Bohan, > > > > this LGTM now. I don't really like that we modify pred_widen_abd_plus > > twice but I guess that's very minor and can be understood as two > > separate fixes so is no blocker. > > > > Please always specify how a patch was tested, we need this for every > > contribution (in particular as the CI is down right now). > Right. For the wider audience (and since we didn't have the patchwork > call yesterday). I looked into this last week, it appears the > pre-commit system was pulling the newlib repository from a personal > mirror rather than the official repository. That personal mirror seems > to have disappeared and as such the pre-commit CI system fails to > checkout the repo and thus doesn't run any builds/tests. > > I've submitted a MR that has a reasonable chance to fix the problem, but > the owner of the pre-commit CI system hasn't responded. There is an > effort to move that pre-commit system into the RISE area of github > (where I could fix it directly rather than waiting on someone else), but > that transition hasn't been completed. > > While we get this situation under control, it is helpful if everyone is > more diligent about testing their RISC-V patches. While we still have > two post-commit CI systems running, it's obviously better to catch > problems before they get into the tree. > > Thanks, > jeff
I regtested the whole testsuite with rv64gcv-lp64d-rocket and 32gcv-ilp32d-rocket; no regression. I just found that the current intrinsic doc of Zvabd (https://github.com/riscv-non-isa/riscv-rvv-intrinsic-doc) is inconsistent with the Zvabd spec (https://github.com/riscv/integer-vector-absolute-difference) about the operand order of vwabda[u]. The intrinsic doc uses `vs1, vs2` but the extension spec uses `vs2, vs1`. For an instruction like vwabda it should be semantically equivalent, though. I also checked the LLVM implementation of the intrinsics, and found that the first source argument was used for vs2 in the assembly, identical to my implementation, though the name of the intrinsic argument was `vs1` in the clang tests. I already created a PR in the intrinsic doc repo. Shall we wait for that, or can we just go ahead and push this now? Thanks, Bohan
