Looking through svn log lib/Target/ARM/ | less, maybe r272251?

Yup, reverting that locally makes the assert go away. James, does this look like an easy fix, or should we revert?

Also, do you see an easy workaround around this crash until we can deploy a fixed compiler?

Workaround: Use volatile int kPad = 4095; and then s/4095/kPad/ :-P

Workaround: Use volatile int kPad = 4095; and then s/4095/kPad/ :-P

Hum, this sounds like constant folding / CSE, and not a case Jame's patch [1] was testing for.

Probably this has provided a case which he wasn't expecting, and the testing wasn't really that thorough... :/

Maybe reverting the patch for now would be a good thing, unless James has a quick fix (and more tests!).

cheers,
--renato

[1] http://llvm.org/viewvc/llvm-project?view=revision&revision=272251

Hm, this might fix it:

Index: lib/Target/ARM/ARMISelDAGToDAG.cpp

--- lib/Target/ARM/ARMISelDAGToDAG.cpp (revision 274127)
+++ lib/Target/ARM/ARMISelDAGToDAG.cpp (working copy)
@@ -2835,11 +2835,11 @@
Subtarget->hasThumb2() && !is_t2_so_imm(Imm) && is_t2_so_imm_not(Imm);
if (!PreferImmediateEncoding &&
ConstantMaterializationCost(Imm) >

•          ConstantMaterializationCost(~Imm)) {
  

•          ConstantMaterializationCost(-Imm)) {
     // The current immediate costs more to materialize than a negated
     // immediate, so negate the immediate and use a BIC.
     SDValue NewImm =
  

•        CurDAG->getConstant(~N1C->getZExtValue(), dl, MVT::i32);
  

•        CurDAG->getConstant(-N1C->getZExtValue(), dl, MVT::i32);
     CurDAG->RepositionNode(N->getIterator(), NewImm.getNode());
  
     if (!Subtarget->hasThumb2()) {
  

Hm no, that's not good, BIC is AND NOT.

$ svn commit -m 'Revert r272251, it caused #28722 .'
Sending lib/Target/ARM/ARMISelDAGToDAG.cpp
Deleting test/CodeGen/Thumb/bic_imm.ll
Transmitting file data .
Committed revision 274141.

$ svn commit -m 'Add a regression test for #28722 .'
Adding test/CodeGen/Thumb/and_neg.ll
Transmitting file data .
Committed revision 274142.

reduced.c
Add an additional, slightly larger test case.

Hi Nico,

Thanks for bringing this to my attention, and sorry that I didn't act on it quickly as I was on vacation.

The underlying problem is actually my use of DAG->getConstant. I should be using DAG->getTargetConstant.

The use of getConstant() in this testcase actually increments the use count of Constant<4095> from one to two. In this case, the lowering of ARMISD::BFI is contingent on its constant argument only having one use - if it's shared, ISel falls over.

I've committed a fixed version in r274510.

Cheers,

James