SjoerdMeijer added a comment. Bit of a drive-by comment, but I can't say I am big fan of all the string matching on the register names. Not sure if this is a fair comment, because I haven't looked closely at it yet, but could we use more the `ARM::R[0-9]` values more? Perhaps that's difficult from the Clang parts?
================ Comment at: clang/lib/Basic/Targets/ARM.cpp:902 + std::vector<std::string> &Features = getTargetOpts().Features; + std::string SearchFeature = "+reserve-" + RegName.str(); + for (std::string &Feature : Features) { ---------------- I was pointed at something similar myself recently, but if I am not mistaken then I think this is a use-after-free: "+reserve-" + RegName.str() this will allocate a temp `std::string` that `SearchFeature` points to, which then gets released, and `SearchFeature` is still pointing at it. ================ Comment at: llvm/test/CodeGen/ARM/reg-alloc-with-fixed-reg-r6-modified.ll:15 +; r6 = 10; +; unsigned int result = i + j + k + l +m + n + o + p; +; } ---------------- nit: `+m` -> ` + m` ================ Comment at: llvm/test/CodeGen/ARM/reg-alloc-with-fixed-reg-r6.ll:13 +; { +; unsigned int result = i + j + k + l +m + n + o + p; +; } ---------------- same nit ================ Comment at: llvm/test/CodeGen/ARM/reg-alloc-wout-fixed-regs.ll:3 +; +; Equivalent C source code +; void bar(unsigned int i, ---------------- As all these tests (this file and the ones above) are the same, the "equivalent C source code" is the same, perhaps move all these cases into 1 file. ================ Comment at: llvm/test/CodeGen/ARM/reg-alloc-wout-fixed-regs.ll:13 +; { +; unsigned int result = i + j + k + l +m + n + o + p; +; } ---------------- same nit here Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68862/new/ https://reviews.llvm.org/D68862 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits