Optimizer emitting wrong alignment for atomic store

IR & Optimizations
1

I was trying to compile lldb today using the latest clang for i686 and it failed at linking step due to undefined symbol __atomic_store and I don’t have libatomic.

Upon closer look, the code which caused call __atomic_store to be emitted is at lldb/source/Utility/Log.cpp:108

   105	void Log::Disable(uint32_t flags) {
   106	  llvm::sys::ScopedWriter lock(m_mutex);
   107	
   108	  MaskType mask = m_mask.fetch_and(~flags, std::memory_order_relaxed);
   109	  if (!(mask & ~flags)) {
   110	    m_handler.reset();
   111	    m_channel.log_ptr.store(nullptr, std::memory_order_relaxed);
   112	  }
   113	}

And the call to Disable() is inlined into the following code.

   203  void Log::Unregister(llvm::StringRef name) {
   204    auto iter = g_channel_map->find(name);
   205    assert(iter != g_channel_map->end());
   206    iter->second.Disable(UINT32_MAX);
   207    g_channel_map->erase(iter);
   208  }

The MaskType is uint64_t so an 8-byte atomic operation is needed. The optimizer figures out that the anding with ~UINT32_MAX will always result to 0 being written, so it’s turned into an atomic store operation. However the wrong alignment is emitted:

store atomic i64 0, ptr %20 monotonic, align 4

Which then results in a call to libatomic. The wrong alignment seems like a bug. If I force Log::Disable() to be not inlined, then this problem goes away.

2

This is happening in InstCombineAtomicRMW.cpp

   103	Instruction *InstCombinerImpl::visitAtomicRMWInst(AtomicRMWInst &RMWI) {
   104	
...
   110	
   111	  // Any atomicrmw op which produces a known result in memory can be
   112	  // replaced w/an atomicrmw xchg.
   113	  if (isSaturating(RMWI) &&
   114	      RMWI.getOperation() != AtomicRMWInst::Xchg) {
   115	    RMWI.setOperation(AtomicRMWInst::Xchg);
   116	    return &RMWI;
   117	  }
...
   124	  // Any atomicrmw xchg with no uses can be converted to a atomic store if the
   125	  // ordering is compatible.
   126	  if (RMWI.getOperation() == AtomicRMWInst::Xchg &&
   127	      RMWI.use_empty()) {
   128	    if (Ordering != AtomicOrdering::Release &&
   129	        Ordering != AtomicOrdering::Monotonic)
   130	      return nullptr;
   131	    auto *SI = new StoreInst(RMWI.getValOperand(),
   132	                             RMWI.getPointerOperand(), &RMWI);
   133	    SI->setAtomic(Ordering, RMWI.getSyncScopeID());
   134	    SI->setAlignment(DL.getABITypeAlign(RMWI.getType()));
   135	    return eraseInstFromFunction(RMWI);
   136	  }

Should line 134 just copy preserve the same alignment as the RMWI instruction? Something like:

   134	    SI->setAlignment(RMWI.getAlign());
3

Yes, it should. This code most likely predates RMW instructions having explicit alignment. We have the same bug later in the file on line 161, creating a LoadInst.