[PATCH] D114611: [AVR] Expand STDWSPQRr & STDSPQRr, approach #2

Dylan McKay via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 30 17:25:57 PST 2021


dylanmckay added inline comments.


================
Comment at: llvm/test/CodeGen/AVR/pseudo/STDWSPQRr.ll:1
+; RUN: llc < %s -march=avr | FileCheck %s
+
----------------
Patryk27 wrote:
> dylanmckay wrote:
> > This test is good but it would be good to also have a `mir` test as well (like the other pseudo instruction tests in the same folder) to make sure that the instructions `STDSPQRr` and `STDWSPQRr` are always covered. Technically, the current test could become a false positive in the future if the instruction selection layer changes and the test passes bypassing the instruction you meant to test. Pretty unlikely in this situation though. But it would still be good to have it just to make sure it is covered with an MIR test too.
> > Technically, the current test could become a false positive in the future if the instruction selection layer changes and the test passes bypassing the instruction you meant to test.
> 
> True, true - how about I move this file to `llvm/test/CodeGen/AVR/bug-$issuenumber.ll` (not to lose the context of "this used to be an edge case"), and then create two separate MIR tests for those opcodes?
Good idea, yup


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114611/new/

https://reviews.llvm.org/D114611



More information about the llvm-commits mailing list