[PATCH] D64124: [PowerPC] Hardware Loop branch instruction's condition may not be icmp

Wed Jul 3 14:30:14 PDT 2019

hfinkel added a comment.

In D64124#1568593 <https://reviews.llvm.org/D64124#1568593>, @samparker wrote:

> Thanks. LGTM

LGTM too, just some comments about the test.

================
Comment at: llvm/test/CodeGen/PowerPC/pr42492.ll:1
+; REQUIRES: asserts
+; RUN: llc -mtriple=powerpc64-unknown-linux-gnu -mcpu=pwr9 < %s | FileCheck %s
----------------
You don't need 'REQUIRES: asserts' here. 'REQUIRES: asserts' is only necessary if asserts are necessary for the test to pass, not if asserts is required for the test to fail. The test still might fail anyway (e.g., it might segfault).

Also, in this case, it seems as though the test can test for correct behavior (not just the absence of a crash), and so please add some CHECK lines to check for the expected output in this case.

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

https://reviews.llvm.org/D64124