Skip to content

Commit

Permalink
[BPF] Improve error message for seq_cst atomic load and store
Browse files Browse the repository at this point in the history
Sequentially consistent (seq_cst) atomic load and store are not
supported yet for BPF.  Right now, calling __atomic_{load,store}{,_n}()
with __ATOMIC_SEQ_CST will cause an error:

  $ cat bar.c
  int foo(int *ptr) { return __atomic_load_n(ptr, __ATOMIC_SEQ_CST); }
  $ clang --target=bpf -mcpu=v5 -c bar.c > /dev/null
  fatal error: error in backend: Cannot select: t8: i32,ch = AtomicLoad<(load seq_cst (s32) from %ir.0)> t7:1, t7
  ...

Which isn't very useful.  Just like commit 379d908 ("BPF: provide
better error message for unsupported atomic operations"), make it
generate an error message saying that the requested operation isn't
supported, before triggering that "fatal error":

  $ clang --target=bpf -mcpu=v5 -c bar.c > /dev/null
  bar.c:1:5: error: sequentially consistent (seq_cst) atomic load is not supported
    1 | int foo(int *ptr) { return __atomic_load_n(ptr, __ATOMIC_SEQ_CST); }
      |     ^
  ...
  • Loading branch information
peilin-ye committed Oct 1, 2024
1 parent 845e062 commit e23e67c
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 1 deletion.
45 changes: 45 additions & 0 deletions llvm/lib/Target/BPF/BPFISelLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ BPFTargetLowering::BPFTargetLowering(const TargetMachine &TM,
setOperationAction(ISD::ATOMIC_CMP_SWAP_WITH_SUCCESS, VT, Custom);
}

for (auto VT : {MVT::i8, MVT::i16, MVT::i32, MVT::i64}) {
setOperationAction(ISD::ATOMIC_LOAD, VT, Custom);
setOperationAction(ISD::ATOMIC_STORE, VT, Custom);
}

for (auto VT : { MVT::i32, MVT::i64 }) {
if (VT == MVT::i32 && !STI.getHasAlu32())
continue;
Expand Down Expand Up @@ -291,6 +296,9 @@ void BPFTargetLowering::ReplaceNodeResults(
else
Msg = "unsupported atomic operation, please use 64 bit version";
break;
case ISD::ATOMIC_LOAD:
case ISD::ATOMIC_STORE:
return;
}

SDLoc DL(N);
Expand All @@ -316,6 +324,10 @@ SDValue BPFTargetLowering::LowerOperation(SDValue Op, SelectionDAG &DAG) const {
return LowerSDIVSREM(Op, DAG);
case ISD::DYNAMIC_STACKALLOC:
return LowerDYNAMIC_STACKALLOC(Op, DAG);
case ISD::ATOMIC_LOAD:
return LowerATOMIC_LOAD(Op, DAG);
case ISD::ATOMIC_STORE:
return LowerATOMIC_STORE(Op, DAG);
}
}

Expand Down Expand Up @@ -703,6 +715,39 @@ SDValue BPFTargetLowering::LowerSELECT_CC(SDValue Op, SelectionDAG &DAG) const {
return DAG.getNode(BPFISD::SELECT_CC, DL, VTs, Ops);
}

SDValue BPFTargetLowering::LowerATOMIC_LOAD(SDValue Op,
SelectionDAG &DAG) const {
const char *Msg =
"sequentially consistent (seq_cst) atomic load is not supported";
SDNode *N = Op.getNode();
SDLoc DL(N);

if (cast<AtomicSDNode>(N)->getMergedOrdering() ==
AtomicOrdering::SequentiallyConsistent)
fail(DL, DAG, Msg);

return Op;
}

SDValue BPFTargetLowering::LowerATOMIC_STORE(SDValue Op,
SelectionDAG &DAG) const {
const char *Msg =
"sequentially consistent (seq_cst) atomic store is not supported";
EVT VT = Op.getOperand(1).getValueType();
SDNode *N = Op.getNode();
SDLoc DL(N);

// Promote operand #1 (value to store) if necessary.
if (!isTypeLegal(VT))
return SDValue();

if (cast<AtomicSDNode>(N)->getMergedOrdering() ==
AtomicOrdering::SequentiallyConsistent)
fail(DL, DAG, Msg);

return Op;
}

const char *BPFTargetLowering::getTargetNodeName(unsigned Opcode) const {
switch ((BPFISD::NodeType)Opcode) {
case BPFISD::FIRST_NUMBER:
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/BPF/BPFISelLowering.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ class BPFTargetLowering : public TargetLowering {
SDValue LowerDYNAMIC_STACKALLOC(SDValue Op, SelectionDAG &DAG) const;
SDValue LowerBR_CC(SDValue Op, SelectionDAG &DAG) const;
SDValue LowerSELECT_CC(SDValue Op, SelectionDAG &DAG) const;

SDValue LowerATOMIC_LOAD(SDValue Op, SelectionDAG &DAG) const;
SDValue LowerATOMIC_STORE(SDValue Op, SelectionDAG &DAG) const;
SDValue LowerConstantPool(SDValue Op, SelectionDAG &DAG) const;
SDValue LowerGlobalAddress(SDValue Op, SelectionDAG &DAG) const;

Expand Down

0 comments on commit e23e67c

Please sign in to comment.