MaskRay added inline comments.

================
Comment at: llvm/test/CodeGen/RISCV/loop-alignment.ll:3-4
+; RUN: llc < %s -mtriple=riscv64 | FileCheck %s
+; RUN: llc < %s -mtriple=riscv64 -align-loops=16 | FileCheck %s 
-check-prefix=ALIGN_16
+; RUN: llc < %s -mtriple=riscv64 -align-loops=32 | FileCheck %s 
-check-prefix=ALIGN_32
+
----------------
luismarques wrote:
> MaskRay wrote:
> > jrtc27 wrote:
> > > MaskRay wrote:
> > > > jrtc27 wrote:
> > > > > MaskRay wrote:
> > > > > > luismarques wrote:
> > > > > > > Nit: it's a convention of the RISC-V backend codegen tests to 
> > > > > > > wrap the RUN lines.
> > > > > > only 86 columns. compiler-rt is even transiting to 100 column.
> > > > > compiler-rt is not the RISC-V backend :)
> > > > Wrapping lines here just makes the code less readable.
> > > That's your personal opinion, which I disagree with, and it's not true if 
> > > your terminal isn't wide enough. Going against existing convention in the 
> > > backend tests should only be done with very good reason, and personal 
> > > opinion is not that.
> > Lines longer than 80-column (in this case just 86) are pretty common among 
> > tests. I really hope test/CodeGen/RISCV/ can be more tolerant on this 
> > matter.
> > 
> > Even the Linux scripts/checkpatch.pl has increased the limit to 100 because 
> > in many cases wrapping lines for strict 80-conformance just harms 
> > readability.
> > 
> > Of course I don't want to waste time arguing on this matter. So if this 
> > turns out to be an issue for RISC-V folks, I'll update it to save my time.
> > Of course I don't want to waste time arguing on this matter. So if this 
> > turns out to be an issue for RISC-V folks, I'll update it to save my time.
> 
> Personally, I don't particularly care. I don't know if @asb has strong 
> feelings about this. If you think it would be beneficial to relax this 
> convention please raise the issue on llvm-dev. Let's not keep discussing this 
> in every patch touching RISC-V :-)
Personally I don't even think the generic case needs to be raised on llvm-dev:) 
There are just so many column>80 cases in llvm/test and clang/test. Actually, 
If someone wants to enforce the 80-column rule more rigidly, that probably 
needs a discussion.

That said, the argument here is about a subdirectory: llvm/test/CodeGen/RISCV/ 
...




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106701

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to